fix(skills): skip search results that fail frontmatter validation

`GCPSkillRegistry.search_skills` built a `Frontmatter` for every hit with no
per-item error handling. `Frontmatter.name` must be kebab-case (or snake_case
behind the feature flag), but the catalog holds names outside that set -- the
first-party entry `cloud.google.com-agent-platform-eval-flywheel` has dots. The
first such hit raised a pydantic `ValidationError` and took down the whole
call, so search returned nothing at all against any catalog that holds one
non-conforming entry. An entry with an empty description did the same.

Skip the entry and log a warning instead. The caller does not control what the
catalog holds, so one entry it never asked about must not break discovery for
everything else.

A name that is not a string gets the same treatment. `.split` on it raises
before validation is reached, which would sink the call the same way.

Fixes #6838

Co-authored-by: Kathy Wu <wukathy@google.com>
PiperOrigin-RevId: 968161389

src/google/adk/integrations/skill_registry/gcp_skill_registry.py

@@ -17,6 +17,7 @@
 from __future__ import annotations
 
 import asyncio
+import logging
 import os
 import ssl
 import tempfile
@@ -35,6 +36,9 @@ import google.auth.exceptions
 from google.auth.transport import mtls
 from google.auth.transport import requests as auth_requests
 import httpx
+from pydantic import ValidationError
+
+logger = logging.getLogger("google_adk." + __name__)
 
 
 class GCPSkillRegistry(SkillRegistry):
@@ -219,7 +223,10 @@ class GCPSkillRegistry(SkillRegistry):
       query: The search query.
 
     Returns:
-      A list of Frontmatter objects for discovery.
+      A list of Frontmatter objects for discovery. A catalog entry that fails
+      client-side frontmatter validation is skipped and logged, not raised: the
+      caller does not control what the catalog holds, so one entry it never
+      asked about must not break discovery for everything else.
     """
     async with self._create_httpx_client() as client:
       url = (
@@ -234,10 +241,23 @@ class GCPSkillRegistry(SkillRegistry):
 
       results = []
       for s in response_data.get("skills", []):
-        results.append(
-            models.Frontmatter(
-                name=s.get("name", "").split("/")[-1],
-                description=s.get("description", "") or "",
-            )
-        )
+        # A non-string name is as much outside the caller's control as a
+        # non-conforming one, so give it the same treatment: an empty name
+        # fails validation below and takes the skip path.
+        raw_name = s.get("name")
+        name = raw_name.split("/")[-1] if isinstance(raw_name, str) else ""
+        try:
+          results.append(
+              models.Frontmatter(
+                  name=name,
+                  description=s.get("description", "") or "",
+              )
+          )
+        except ValidationError as e:
+          logger.warning(
+              "Skipping search result %r: it does not pass frontmatter"
+              " validation: %s",
+              name,
+              e,
+          )
       return results

tests/unittests/integrations/skill_registry/test_gcp_skill_registry.py

@@ -15,6 +15,7 @@
 """Tests for GCP Skill Registry."""
 
 import io
+import logging
 import os
 from unittest import mock
 import zipfile
@@ -184,6 +185,92 @@ async def test_search_skills_success():
   )
 
 
+@pytest.mark.parametrize(
+    "bad_name, bad_description",
+    [
+        # A real first-party catalog entry: dots are outside the name pattern.
+        ("cloud.google.com-agent-platform-eval-flywheel", "Description bad"),
+        ("Skill-With-Caps", "Description bad"),
+        ("a" * 65, "Description bad"),
+        ("skill-no-description", ""),
+    ],
+)
+@pytest.mark.asyncio
+async def test_search_skills_skips_entry_failing_validation(
+    caplog, bad_name, bad_description
+):
+  """A catalog entry the client cannot represent must not sink the search.
+
+  The caller does not control what the catalog holds, so one entry that fails
+  frontmatter validation has to be skipped, leaving every valid hit returned.
+  Skipping loses data, so the warning is part of the contract: it is the only
+  signal the caller gets that a hit was dropped.
+  """
+  registry = gcp_skill_registry.GCPSkillRegistry()
+
+  mock_response = mock.MagicMock()
+  mock_response.status_code = 200
+  mock_response.json.return_value = {
+      "skills": [
+          {
+              "name": (
+                  f"projects/test-project/locations/us-central1/skills/{bad_name}"
+              ),
+              "description": bad_description,
+          },
+          {
+              "name": (
+                  "projects/test-project/locations/us-central1/skills/skill2"
+              ),
+              "description": "Description 2",
+          },
+      ]
+  }
+
+  with mock.patch("httpx.AsyncClient.get", return_value=mock_response):
+    with caplog.at_level(logging.WARNING, logger="google_adk"):
+      results = await registry.search_skills(query="query")
+
+  assert [r.name for r in results] == ["skill2"]
+  assert results[0].description == "Description 2"
+  assert len(caplog.records) == 1
+  assert bad_name in caplog.text
+
+
+@pytest.mark.parametrize("raw_name", [None, 7, ["a"]])
+@pytest.mark.asyncio
+async def test_search_skills_skips_entry_whose_name_is_not_a_string(
+    caplog, raw_name
+):
+  """A name that is not a string must take the same skip path.
+
+  `.split` on a non-string raises before validation is ever reached, which
+  would take down the whole call again -- the exact failure this skip removes.
+  """
+  registry = gcp_skill_registry.GCPSkillRegistry()
+
+  mock_response = mock.MagicMock()
+  mock_response.status_code = 200
+  mock_response.json.return_value = {
+      "skills": [
+          {"name": raw_name, "description": "Description 1"},
+          {
+              "name": (
+                  "projects/test-project/locations/us-central1/skills/skill2"
+              ),
+              "description": "Description 2",
+          },
+      ]
+  }
+
+  with mock.patch("httpx.AsyncClient.get", return_value=mock_response):
+    with caplog.at_level(logging.WARNING, logger="google_adk"):
+      results = await registry.search_skills(query="query")
+
+  assert [r.name for r in results] == ["skill2"]
+  assert len(caplog.records) == 1
+
+
 @pytest.mark.asyncio
 async def test_registry_requests_identify_adk():
   """Registry calls carry the ADK client label.