fix(skills): accept dotted registry ids in GCPSkillRegistry

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

@@ -19,6 +19,7 @@
 import asyncio
 import logging
 import os
+import re
 import ssl
 import tempfile
 from typing import Any
@@ -36,10 +37,22 @@
 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__)
 
+# Registry resource ids (e.g. "cloud.google.com-agent-platform-eval-flywheel"
+# for Google-published skills) are a different namespace from SKILL.md
+# frontmatter names: they are not required to be kebab/snake-case and may
+# contain dots. They still need to be safe to interpolate as a single URL
+# path segment, so they get their own, more permissive check instead of
+# reusing the SKILL.md content naming rule.
+_SAFE_REGISTRY_ID_PATTERN = re.compile(r"^[a-z0-9]+(?:[._-][a-z0-9]+)*$")
+
+
+def _is_safe_registry_id(name: str) -> bool:
+  """True if `name` is safe to use as a single skill-registry path segment."""
+  return len(name) <= 256 and bool(_SAFE_REGISTRY_ID_PATTERN.match(name))
+
 
 class GCPSkillRegistry(SkillRegistry):
   """GCP implementation of SkillRegistry using GCP Skill Registry API."""
@@ -175,15 +188,16 @@ async def get_skill(self, *, name: str) -> models.Skill:
       ValueError: If the name is not a valid skill name.
     """
     # The name reaches here straight from a model-issued tool call, so it must
-    # be a single path segment before it is interpolated into the request URL.
-    # Accept the same character set skill names are already held to; the
-    # snake-or-kebab pattern is the superset of the two accepted spellings.
-    # pylint: disable-next=protected-access
-    if not models._SNAKE_OR_KEBAB_NAME_PATTERN.match(name):
+    # be a single, safe path segment before it is interpolated into the
+    # request URL. This is a registry resource id, not a SKILL.md frontmatter
+    # name, so it is held to its own safe-path-segment rule rather than the
+    # stricter kebab/snake-case naming rule SKILL.md content is held to.
+    if not _is_safe_registry_id(name):
       raise ValueError(
-          f"Invalid skill name {name!r}: name must be lowercase kebab-case"
-          " (a-z, 0-9, hyphens) or snake_case (a-z, 0-9, underscores), with"
-          " no leading, trailing, or consecutive delimiters."
+          f"Invalid skill name {name!r}: name must be a single safe path"
+          " segment of at most 256 characters (lowercase letters, digits,"
+          " and non-consecutive '.', '_', '-' separators), with no leading,"
+          " trailing, or consecutive delimiters."
       )
 
     async with self._create_httpx_client() as client:
@@ -246,18 +260,30 @@ async def search_skills(self, *, query: str) -> list[models.Frontmatter]:
         # fails validation below and takes the skip path.
         raw_name = s.get("name")
         name = raw_name.split("/")[-1] if isinstance(raw_name, str) else ""
+        # A registry id is not a SKILL.md frontmatter name (see get_skill),
+        # so it is checked against the safe-path-segment rule instead of
+        # Frontmatter's stricter kebab/snake-case name validator.
+        if not _is_safe_registry_id(name):
+          logger.warning(
+              "Skipping search result %r: not a safe registry id.", name
+          )
+          continue
         try:
-          results.append(
-              models.Frontmatter(
-                  name=name,
-                  description=s.get("description", "") or "",
-              )
+          # pylint: disable-next=protected-access
+          description = models.Frontmatter._validate_description(
+              s.get("description", "") or ""
           )
-        except ValidationError as e:
+        except ValueError as e:
           logger.warning(
               "Skipping search result %r: it does not pass frontmatter"
               " validation: %s",
               name,
               e,
           )
+          continue
+        results.append(
+            models.Frontmatter.model_construct(
+                name=name, description=description
+            )
+        )
       return results

tests/unittests/integrations/skill_registry/test_gcp_skill_registry.py

@@ -188,10 +188,11 @@ 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"),
+        # A bare traversal segment must still be rejected even though '.' is
+        # otherwise an allowed registry-id character.
+        ("..", "Description bad"),
         ("Skill-With-Caps", "Description bad"),
-        ("a" * 65, "Description bad"),
+        ("a" * 257, "Description bad"),
         ("skill-no-description", ""),
     ],
 )
@@ -237,6 +238,37 @@ async def test_search_skills_skips_entry_failing_validation(
   assert bad_name in caplog.text
 
 
+@pytest.mark.asyncio
+async def test_search_skills_accepts_dotted_registry_id():
+  """A Google-published registry id with dots must not be dropped.
+
+  Regression test for https://github.com/google/adk-python/issues/7136:
+  ids like "cloud.google.com-<name>" are registry resource ids, not
+  SKILL.md frontmatter names, so they must not be checked against the
+  stricter kebab/snake-case frontmatter naming rule.
+  """
+  registry = gcp_skill_registry.GCPSkillRegistry()
+
+  mock_response = mock.MagicMock()
+  mock_response.status_code = 200
+  mock_response.json.return_value = {
+      "skills": [{
+          "name": (
+              "projects/test-project/locations/global/skills/"
+              "cloud.google.com-agent-platform-eval-flywheel"
+          ),
+          "description": "A Google-published skill.",
+      }]
+  }
+
+  with mock.patch("httpx.AsyncClient.get", return_value=mock_response):
+    results = await registry.search_skills(query="query")
+
+  assert len(results) == 1
+  assert results[0].name == "cloud.google.com-agent-platform-eval-flywheel"
+  assert results[0].description == "A Google-published skill."
+
+
 @pytest.mark.parametrize("raw_name", [None, 7, ["a"]])
 @pytest.mark.asyncio
 async def test_search_skills_skips_entry_whose_name_is_not_a_string(
@@ -388,6 +420,9 @@ async def mock_get(url, *unused_args, **kwargs):
         "my-skill/revisions/rev-123",
         "My-Skill",
         "",
+        ".",
+        "..",
+        "a" * 257,
     ],
 )
 @pytest.mark.asyncio
@@ -402,7 +437,18 @@ async def test_get_skill_rejects_unsafe_name_before_any_request(unsafe_name):
   mock_get_called.assert_not_called()
 
 
-@pytest.mark.parametrize("valid_name", ["my-skill", "my_skill", "skill2"])
+@pytest.mark.parametrize(
+    "valid_name",
+    [
+        "my-skill",
+        "my_skill",
+        "skill2",
+        "cloud.google.com-agent-platform-eval-flywheel",
+        # Real catalog ids longer than the old 64-char cap (80 and 65 chars).
+        "cloud.google.com-google-cloud-solution-agentic-analytics-spark-knowledge-catalog",
+        "cloud.google.com-gke-ai-troubleshooting-handle-disruption-gpu-tpu",
+    ],
+)
 @pytest.mark.asyncio
 async def test_get_skill_builds_expected_url_for_valid_name(valid_name):
   """Verifies that a valid name is still interpolated verbatim into the URL."""