Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/google/adk/skills/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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"
)
Expand Down
84 changes: 84 additions & 0 deletions tests/unittests/skills/test__utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,90 @@ def test__list_skills_in_gcs_dir(mock_client_class):
assert skills["my-skill"].name == "my-skill"


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()
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):
name = path.rsplit("/", 1)[-1]
m = mock.MagicMock()
m.exists.return_value = name in manifests
m.download_as_text.return_value = (
f"---\nname: my-skill\ndescription: A skill\n---\n{manifests.get(name)}"
)
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_manifests(mock_client_class, {"skill.md": "Body"})

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_manifests(
mock_client_class, {"skill.md": "Body"}
)

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",
]


@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()
Expand Down
Loading