From 5b59e893f3f25b159852eef1255dae82251bba12 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Thu, 29 Oct 2026 12:12:33 -0700 Subject: [PATCH 1/2] fix(skills): accept a skill.md manifest in GCS skill directories The local directory and zip loaders accept SKILL.md or skill.md, but _list_skills_in_gcs_dir and _load_skill_from_gcs_dir only looked for SKILL.md, so a skill that loads locally vanished from the listing and failed to load once stored in a bucket. Look up the manifest under both names, in the same order the other loaders use. Fixes #7842 --- src/google/adk/skills/_utils.py | 25 ++++++++++-- tests/unittests/skills/test__utils.py | 55 +++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/google/adk/skills/_utils.py b/src/google/adk/skills/_utils.py index 66d89306cae..44f00bdc489 100644 --- a/src/google/adk/skills/_utils.py +++ b/src/google/adk/skills/_utils.py @@ -675,9 +675,9 @@ def _list_skills_in_gcs_dir( skills: dict[str, models.Frontmatter] = {} for skill_prefix in sorted(iterator.prefixes): - manifest_blob = bucket.blob(f"{skill_prefix}SKILL.md") + manifest_blob = _find_gcs_skill_manifest(bucket, skill_prefix) - if manifest_blob.exists(): + if manifest_blob is not None: content = manifest_blob.download_as_text() skill_id = skill_prefix.rstrip("/").split("/")[-1] try: @@ -698,6 +698,23 @@ def _list_skills_in_gcs_dir( return skills +def _find_gcs_skill_manifest(bucket: Any, skill_prefix: str) -> Any: + """Returns the blob holding a GCS skill's manifest, or None if it has none. + + Accepts the same manifest names as the local and zip loaders, in the same + order, so a skill directory loads the same way wherever it is stored. + + Args: + bucket: The GCS bucket that holds the skill. + skill_prefix: The skill directory's object prefix, ending in "/". + """ + for name in ("SKILL.md", "skill.md"): + blob = bucket.blob(f"{skill_prefix}{name}") + if blob.exists(): + return blob + return None + + def _load_skill_from_gcs_dir( bucket_name: str, skill_id: str, @@ -750,9 +767,9 @@ def _load_skill_from_gcs_dir( base_prefix += "/" skill_dir_prefix = f"{base_prefix}{skill_id}/" - manifest_blob = bucket.blob(f"{skill_dir_prefix}SKILL.md") + manifest_blob = _find_gcs_skill_manifest(bucket, skill_dir_prefix) - if not manifest_blob.exists(): + if manifest_blob is None: raise FileNotFoundError( f"SKILL.md not found at gs://{bucket_name}/{skill_dir_prefix}SKILL.md" ) diff --git a/tests/unittests/skills/test__utils.py b/tests/unittests/skills/test__utils.py index 4178ba95d21..6e6d436b242 100644 --- a/tests/unittests/skills/test__utils.py +++ b/tests/unittests/skills/test__utils.py @@ -419,6 +419,61 @@ def test__list_skills_in_gcs_dir(mock_client_class): assert skills["my-skill"].name == "my-skill" +def _mock_gcs_bucket_with_lowercase_manifest(mock_client_class): + """Wires a mocked GCS client to a skill whose manifest is skill.md.""" + mock_client = mock.MagicMock() + mock_client_class.return_value = mock_client + mock_bucket = mock.MagicMock() + mock_client.bucket.return_value = mock_bucket + + mock_iterator = mock.MagicMock() + mock_iterator.prefixes = ["skills/my-skill/"] + + def list_blobs_side_effect(prefix=None, delimiter=None): + return mock_iterator if delimiter == "/" else [] + + mock_bucket.list_blobs.side_effect = list_blobs_side_effect + + def mock_blob_side_effect(path): + m = mock.MagicMock() + m.exists.return_value = path.endswith("/skill.md") + m.download_as_text.return_value = ( + "---\nname: my-skill\ndescription: A skill\n---\nBody" + ) + return m + + mock_bucket.blob.side_effect = mock_blob_side_effect + return mock_bucket + + +@mock.patch("google.cloud.storage.Client") +def test__list_skills_in_gcs_dir_accepts_lowercase_manifest(mock_client_class): + """Like the local and zip loaders, GCS accepts skill.md as the manifest.""" + _mock_gcs_bucket_with_lowercase_manifest(mock_client_class) + + skills = _list_skills_in_gcs_dir("my-bucket", "skills/") + + assert list(skills) == ["my-skill"] + + +@mock.patch("google.cloud.storage.Client") +def test__load_skill_from_gcs_dir_accepts_lowercase_manifest( + mock_client_class, +): + mock_bucket = _mock_gcs_bucket_with_lowercase_manifest(mock_client_class) + + skill = _load_skill_from_gcs_dir( + "my-bucket", "my-skill", skills_base_path="skills/" + ) + + assert skill.name == "my-skill" + assert skill.instructions == "Body" + assert [c.args[0] for c in mock_bucket.blob.call_args_list] == [ + "skills/my-skill/SKILL.md", + "skills/my-skill/skill.md", + ] + + def _mock_gcs_bucket_with_one_invalid_skill(mock_client_class): """Wires a mocked GCS client to a bucket with one valid, one invalid skill.""" mock_client = mock.MagicMock() From 0337a2aebe6c5d7fdb4f4728cad324fc2edfd6ab Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 30 Oct 2026 18:56:09 -0700 Subject: [PATCH 2/2] test(skills): check that SKILL.md still wins over skill.md in GCS With both manifests in a GCS skill directory, the loader reads SKILL.md and never looks up skill.md, as before. --- tests/unittests/skills/test__utils.py | 41 +++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/tests/unittests/skills/test__utils.py b/tests/unittests/skills/test__utils.py index 6e6d436b242..73b981a3b26 100644 --- a/tests/unittests/skills/test__utils.py +++ b/tests/unittests/skills/test__utils.py @@ -419,8 +419,14 @@ def test__list_skills_in_gcs_dir(mock_client_class): assert skills["my-skill"].name == "my-skill" -def _mock_gcs_bucket_with_lowercase_manifest(mock_client_class): - """Wires a mocked GCS client to a skill whose manifest is skill.md.""" +def _mock_gcs_bucket_with_manifests(mock_client_class, manifests): + """Wires a mocked GCS client to a skill with the given manifest files. + + Args: + mock_client_class: The patched google.cloud.storage.Client. + manifests: Maps each manifest file name in the skill directory to the + instructions body it holds. + """ mock_client = mock.MagicMock() mock_client_class.return_value = mock_client mock_bucket = mock.MagicMock() @@ -435,10 +441,11 @@ def list_blobs_side_effect(prefix=None, delimiter=None): mock_bucket.list_blobs.side_effect = list_blobs_side_effect def mock_blob_side_effect(path): + name = path.rsplit("/", 1)[-1] m = mock.MagicMock() - m.exists.return_value = path.endswith("/skill.md") + m.exists.return_value = name in manifests m.download_as_text.return_value = ( - "---\nname: my-skill\ndescription: A skill\n---\nBody" + f"---\nname: my-skill\ndescription: A skill\n---\n{manifests.get(name)}" ) return m @@ -449,7 +456,7 @@ def mock_blob_side_effect(path): @mock.patch("google.cloud.storage.Client") def test__list_skills_in_gcs_dir_accepts_lowercase_manifest(mock_client_class): """Like the local and zip loaders, GCS accepts skill.md as the manifest.""" - _mock_gcs_bucket_with_lowercase_manifest(mock_client_class) + _mock_gcs_bucket_with_manifests(mock_client_class, {"skill.md": "Body"}) skills = _list_skills_in_gcs_dir("my-bucket", "skills/") @@ -460,7 +467,9 @@ def test__list_skills_in_gcs_dir_accepts_lowercase_manifest(mock_client_class): def test__load_skill_from_gcs_dir_accepts_lowercase_manifest( mock_client_class, ): - mock_bucket = _mock_gcs_bucket_with_lowercase_manifest(mock_client_class) + mock_bucket = _mock_gcs_bucket_with_manifests( + mock_client_class, {"skill.md": "Body"} + ) skill = _load_skill_from_gcs_dir( "my-bucket", "my-skill", skills_base_path="skills/" @@ -474,6 +483,26 @@ def test__load_skill_from_gcs_dir_accepts_lowercase_manifest( ] +@mock.patch("google.cloud.storage.Client") +def test__load_skill_from_gcs_dir_prefers_uppercase_manifest( + mock_client_class, +): + """With both manifests present, SKILL.md is used and skill.md is ignored.""" + mock_bucket = _mock_gcs_bucket_with_manifests( + mock_client_class, + {"SKILL.md": "Upper body", "skill.md": "Lower body"}, + ) + + skill = _load_skill_from_gcs_dir( + "my-bucket", "my-skill", skills_base_path="skills/" + ) + + assert skill.instructions == "Upper body" + assert [c.args[0] for c in mock_bucket.blob.call_args_list] == [ + "skills/my-skill/SKILL.md", + ] + + def _mock_gcs_bucket_with_one_invalid_skill(mock_client_class): """Wires a mocked GCS client to a bucket with one valid, one invalid skill.""" mock_client = mock.MagicMock()