Skip to content

Commit b11b4e3

Browse files
Davide Gallitelliclaude
andcommitted
simplify(skills): inline URL fetch into Skill, remove _url_loader.py
Per maintainer feedback: - Remove _url_loader.py — inline fetch logic directly into Skill.from_url() - Move imports to top of module (not inside methods) - Use source.startswith("https://") directly in _resolve_skills() - Remove AGENTS.md entry for deleted file The entire URL loading feature is now ~15 lines in skill.py with no separate module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent bfa2951 commit b11b4e3

7 files changed

Lines changed: 75 additions & 209 deletions

File tree

AGENTS.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ strands-agents/
141141
│ │ │ ├── core/ # Base classes, actions, context
142142
│ │ │ └── handlers/ # Handler implementations (e.g., LLM)
143143
│ │ └── skills/ # AgentSkills.io integration (Skill, AgentSkills)
144-
│ │ └── _url_loader.py # HTTPS skill fetching, GitHub URL resolution
145144
│ │
146145
│ ├── experimental/ # Experimental features (API may change)
147146
│ │ ├── agent_config.py # Experimental agent config

src/strands/vended_plugins/skills/_url_loader.py

Lines changed: 0 additions & 63 deletions
This file was deleted.

src/strands/vended_plugins/skills/agent_skills.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,16 +295,14 @@ def _resolve_skills(self, sources: list[SkillSource]) -> dict[str, Skill]:
295295
Returns:
296296
Dict mapping skill names to Skill instances.
297297
"""
298-
from ._url_loader import is_url
299-
300298
resolved: dict[str, Skill] = {}
301299

302300
for source in sources:
303301
if isinstance(source, Skill):
304302
if source.name in resolved:
305303
logger.warning("name=<%s> | duplicate skill name, overwriting previous skill", source.name)
306304
resolved[source.name] = source
307-
elif isinstance(source, str) and is_url(source):
305+
elif isinstance(source, str) and source.startswith("https://"):
308306
try:
309307
skill = Skill.from_url(source, strict=self._strict)
310308
if skill.name in resolved:

src/strands/vended_plugins/skills/skill.py

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
"""Skill data model and loading utilities for AgentSkills.io skills.
22
33
This module defines the Skill dataclass and provides classmethods for
4-
discovering, parsing, and loading skills from the filesystem or raw content.
5-
Skills are directories containing a SKILL.md file with YAML frontmatter
6-
metadata and markdown instructions.
4+
discovering, parsing, and loading skills from the filesystem, raw content,
5+
or HTTPS URLs. Skills are directories containing a SKILL.md file with YAML
6+
frontmatter metadata and markdown instructions.
77
"""
88

99
from __future__ import annotations
1010

1111
import logging
1212
import re
13+
import urllib.error
14+
import urllib.request
1315
from dataclasses import dataclass, field
1416
from pathlib import Path
1517
from typing import Any
@@ -359,12 +361,20 @@ def from_url(cls, url: str, *, strict: bool = False) -> Skill:
359361
ValueError: If ``url`` is not an ``https://`` URL.
360362
RuntimeError: If the SKILL.md content cannot be fetched.
361363
"""
362-
from ._url_loader import fetch_skill_content, is_url
363-
364-
if not is_url(url):
364+
if not url.startswith("https://"):
365365
raise ValueError(f"url=<{url}> | not a valid HTTPS URL")
366366

367-
content = fetch_skill_content(url)
367+
logger.info("url=<%s> | fetching skill content", url)
368+
369+
try:
370+
req = urllib.request.Request(url, headers={"User-Agent": "strands-agents-sdk"}) # noqa: S310
371+
with urllib.request.urlopen(req, timeout=30) as response: # noqa: S310
372+
content: str = response.read().decode("utf-8")
373+
except urllib.error.HTTPError as e:
374+
raise RuntimeError(f"url=<{url}> | HTTP {e.code}: {e.reason}") from e
375+
except urllib.error.URLError as e:
376+
raise RuntimeError(f"url=<{url}> | failed to fetch skill: {e.reason}") from e
377+
368378
return cls.from_content(content, strict=strict)
369379

370380
@classmethod

tests/strands/vended_plugins/skills/test_agent_skills.py

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -664,15 +664,25 @@ def test_resolve_nonexistent_path(self, tmp_path):
664664
class TestResolveUrlSkills:
665665
"""Tests for _resolve_skills with URL sources."""
666666

667-
_URL_LOADER = "strands.vended_plugins.skills._url_loader"
667+
_SKILL_MODULE = "strands.vended_plugins.skills.skill"
668668
_SAMPLE_CONTENT = "---\nname: url-skill\ndescription: A URL skill\n---\n# Instructions\n"
669669

670+
def _mock_urlopen(self, content):
671+
"""Create a mock urlopen context manager returning the given content."""
672+
mock_response = MagicMock()
673+
mock_response.read.return_value = content.encode("utf-8")
674+
mock_response.__enter__ = MagicMock(return_value=mock_response)
675+
mock_response.__exit__ = MagicMock(return_value=False)
676+
return mock_response
677+
670678
def test_resolve_url_source(self):
671679
"""Test resolving a URL string as a skill source."""
672680
from unittest.mock import patch
673681

674-
with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT):
675-
plugin = AgentSkills(skills=["https://github.com/org/url-skill"])
682+
with patch(
683+
f"{self._SKILL_MODULE}.urllib.request.urlopen", return_value=self._mock_urlopen(self._SAMPLE_CONTENT)
684+
):
685+
plugin = AgentSkills(skills=["https://example.com/SKILL.md"])
676686

677687
assert len(plugin.get_available_skills()) == 1
678688
assert plugin.get_available_skills()[0].name == "url-skill"
@@ -683,10 +693,12 @@ def test_resolve_mixed_url_and_local(self, tmp_path):
683693

684694
_make_skill_dir(tmp_path, "local-skill")
685695

686-
with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT):
696+
with patch(
697+
f"{self._SKILL_MODULE}.urllib.request.urlopen", return_value=self._mock_urlopen(self._SAMPLE_CONTENT)
698+
):
687699
plugin = AgentSkills(
688700
skills=[
689-
"https://github.com/org/url-skill",
701+
"https://example.com/SKILL.md",
690702
str(tmp_path / "local-skill"),
691703
]
692704
)
@@ -698,16 +710,19 @@ def test_resolve_mixed_url_and_local(self, tmp_path):
698710
def test_resolve_url_failure_skips_gracefully(self, caplog):
699711
"""Test that a failed URL fetch is skipped with a warning."""
700712
import logging
713+
import urllib.error
701714
from unittest.mock import patch
702715

703716
with (
704717
patch(
705-
f"{self._URL_LOADER}.fetch_skill_content",
706-
side_effect=RuntimeError("HTTP 404: Not Found"),
718+
f"{self._SKILL_MODULE}.urllib.request.urlopen",
719+
side_effect=urllib.error.HTTPError(
720+
url="https://example.com", code=404, msg="Not Found", hdrs=None, fp=None
721+
),
707722
),
708723
caplog.at_level(logging.WARNING),
709724
):
710-
plugin = AgentSkills(skills=["https://github.com/org/broken"])
725+
plugin = AgentSkills(skills=["https://example.com/broken/SKILL.md"])
711726

712727
assert len(plugin.get_available_skills()) == 0
713728
assert "failed to load skill from URL" in caplog.text

tests/strands/vended_plugins/skills/test_skill.py

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -554,15 +554,25 @@ def test_strict_mode(self):
554554
class TestSkillFromUrl:
555555
"""Tests for Skill.from_url."""
556556

557-
_URL_LOADER = "strands.vended_plugins.skills._url_loader"
558-
557+
_SKILL_MODULE = "strands.vended_plugins.skills.skill"
559558
_SAMPLE_CONTENT = "---\nname: my-skill\ndescription: A remote skill\n---\nRemote instructions.\n"
560559

560+
def _mock_urlopen(self, content):
561+
"""Create a mock urlopen context manager returning the given content."""
562+
from unittest.mock import MagicMock
563+
564+
mock_response = MagicMock()
565+
mock_response.read.return_value = content.encode("utf-8")
566+
mock_response.__enter__ = MagicMock(return_value=mock_response)
567+
mock_response.__exit__ = MagicMock(return_value=False)
568+
return mock_response
569+
561570
def test_from_url_returns_skill(self):
562571
"""Test loading a skill from a URL returns a single Skill."""
563572
from unittest.mock import patch
564573

565-
with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT):
574+
mock_response = self._mock_urlopen(self._SAMPLE_CONTENT)
575+
with patch(f"{self._SKILL_MODULE}.urllib.request.urlopen", return_value=mock_response):
566576
skill = Skill.from_url("https://raw.githubusercontent.com/org/repo/main/SKILL.md")
567577

568578
assert isinstance(skill, Skill)
@@ -581,24 +591,39 @@ def test_from_url_http_rejected(self):
581591
with pytest.raises(ValueError, match="not a valid HTTPS URL"):
582592
Skill.from_url("http://example.com/SKILL.md")
583593

584-
def test_from_url_fetch_failure_raises(self):
585-
"""Test that a fetch failure propagates as RuntimeError."""
594+
def test_from_url_http_error_raises(self):
595+
"""Test that HTTP errors propagate as RuntimeError."""
596+
import urllib.error
586597
from unittest.mock import patch
587598

588599
with patch(
589-
f"{self._URL_LOADER}.fetch_skill_content",
590-
side_effect=RuntimeError("HTTP 404: Not Found"),
600+
f"{self._SKILL_MODULE}.urllib.request.urlopen",
601+
side_effect=urllib.error.HTTPError(
602+
url="https://example.com", code=404, msg="Not Found", hdrs=None, fp=None
603+
),
591604
):
592605
with pytest.raises(RuntimeError, match="HTTP 404"):
593-
Skill.from_url("https://example.com/nonexistent/SKILL.md")
606+
Skill.from_url("https://example.com/SKILL.md")
607+
608+
def test_from_url_network_error_raises(self):
609+
"""Test that network errors propagate as RuntimeError."""
610+
import urllib.error
611+
from unittest.mock import patch
612+
613+
with patch(
614+
f"{self._SKILL_MODULE}.urllib.request.urlopen",
615+
side_effect=urllib.error.URLError("Connection refused"),
616+
):
617+
with pytest.raises(RuntimeError, match="failed to fetch"):
618+
Skill.from_url("https://example.com/SKILL.md")
594619

595620
def test_from_url_strict_mode(self):
596621
"""Test that strict mode is forwarded to from_content."""
597622
from unittest.mock import patch
598623

599624
bad_content = "---\nname: BAD_NAME\ndescription: Bad\n---\nBody."
600625

601-
with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=bad_content):
626+
with patch(f"{self._SKILL_MODULE}.urllib.request.urlopen", return_value=self._mock_urlopen(bad_content)):
602627
with pytest.raises(ValueError):
603628
Skill.from_url("https://example.com/SKILL.md", strict=True)
604629

tests/strands/vended_plugins/skills/test_url_loader.py

Lines changed: 0 additions & 118 deletions
This file was deleted.

0 commit comments

Comments
 (0)