Skip to content

Commit bfa2951

Browse files
Davide Gallitelliclaude
andcommitted
simplify(skills): remove GitHub URL resolution per maintainer feedback
Per review comment, Skill.from_url() should not be opinionated about URL format. Remove all resolve_to_raw_url logic — the method now simply GETs the provided URL and passes content to from_content(). Users are responsible for providing a direct link to raw SKILL.md content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d049e09 commit bfa2951

4 files changed

Lines changed: 42 additions & 215 deletions

File tree

Lines changed: 11 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,19 @@
11
"""Utilities for loading skills from HTTPS URLs.
22
3-
This module provides functions to detect URL-type skill sources, resolve
4-
GitHub web URLs to raw content URLs, and fetch SKILL.md content over HTTPS.
5-
No git dependency or local caching is required.
3+
This module provides functions to detect URL-type skill sources and
4+
fetch SKILL.md content over HTTPS. No git dependency, local caching,
5+
or URL resolution is required — callers provide a direct URL to the
6+
raw SKILL.md content.
67
"""
78

89
from __future__ import annotations
910

1011
import logging
11-
import re
1212
import urllib.error
1313
import urllib.request
1414

1515
logger = logging.getLogger(__name__)
1616

17-
# Matches GitHub /tree/<ref>/path and /blob/<ref>/path URLs.
18-
# e.g. /owner/repo/tree/main/skills/my-skill -> groups: (owner/repo, main, skills/my-skill)
19-
_GITHUB_TREE_PATTERN = re.compile(r"^/([^/]+/[^/]+)/(?:tree|blob)/([^/]+)(?:/(.+?))?/?$")
20-
2117

2218
def is_url(source: str) -> bool:
2319
"""Check whether a skill source string looks like an HTTPS URL.
@@ -34,93 +30,15 @@ def is_url(source: str) -> bool:
3430
return source.startswith("https://")
3531

3632

37-
def resolve_to_raw_url(url: str) -> str:
38-
"""Resolve a GitHub web URL to a raw content URL for SKILL.md.
39-
40-
Supports several GitHub URL patterns and converts them to
41-
``raw.githubusercontent.com`` URLs::
42-
43-
# Repository root (assumes HEAD and SKILL.md at root)
44-
https://github.com/owner/repo
45-
-> https://raw.githubusercontent.com/owner/repo/HEAD/SKILL.md
46-
47-
# Repository root with @ref
48-
https://github.com/owner/repo@v1.0
49-
-> https://raw.githubusercontent.com/owner/repo/v1.0/SKILL.md
50-
51-
# Tree URL pointing to a directory
52-
https://github.com/owner/repo/tree/main/skills/my-skill
53-
-> https://raw.githubusercontent.com/owner/repo/main/skills/my-skill/SKILL.md
54-
55-
# Blob URL pointing to SKILL.md directly
56-
https://github.com/owner/repo/blob/main/skills/my-skill/SKILL.md
57-
-> https://raw.githubusercontent.com/owner/repo/main/skills/my-skill/SKILL.md
58-
59-
Non-GitHub URLs and ``raw.githubusercontent.com`` URLs are returned as-is,
60-
so they must point directly to a raw SKILL.md file.
61-
62-
Args:
63-
url: An HTTPS URL, possibly a GitHub web URL.
64-
65-
Returns:
66-
A URL that can be fetched directly to obtain SKILL.md content.
67-
"""
68-
# Already a raw URL — return as-is
69-
if "raw.githubusercontent.com" in url:
70-
return url
71-
72-
# Not a github.com URL — return as-is (user provides a direct link)
73-
if not url.startswith("https://github.com"):
74-
return url
75-
76-
# Parse the path portion
77-
path_start = len("https://github.com")
78-
path = url[path_start:]
79-
80-
# Handle /tree/<ref>/path and /blob/<ref>/path
81-
tree_match = _GITHUB_TREE_PATTERN.match(path)
82-
if tree_match:
83-
owner_repo = tree_match.group(1)
84-
ref = tree_match.group(2)
85-
subpath = tree_match.group(3) or ""
86-
87-
# If the URL points to a SKILL.md file directly, use it as-is
88-
if subpath.lower().endswith("skill.md"):
89-
return f"https://raw.githubusercontent.com/{owner_repo}/{ref}/{subpath}"
90-
91-
# Otherwise, assume it's a directory and append SKILL.md
92-
if subpath:
93-
return f"https://raw.githubusercontent.com/{owner_repo}/{ref}/{subpath}/SKILL.md"
94-
return f"https://raw.githubusercontent.com/{owner_repo}/{ref}/SKILL.md"
95-
96-
# Handle plain repo URL: /owner/repo or /owner/repo@ref
97-
# Strip leading slash and any trailing slash
98-
clean_path = path.strip("/")
99-
100-
# Check for @ref suffix
101-
if "@" in clean_path:
102-
at_idx = clean_path.rfind("@")
103-
owner_repo = clean_path[:at_idx]
104-
ref = clean_path[at_idx + 1 :]
105-
else:
106-
owner_repo = clean_path
107-
ref = "HEAD"
108-
109-
# Only match owner/repo (exactly two path segments)
110-
if owner_repo.count("/") == 1:
111-
return f"https://raw.githubusercontent.com/{owner_repo}/{ref}/SKILL.md"
112-
113-
# Unrecognised GitHub URL pattern — return as-is
114-
return url
115-
116-
11733
def fetch_skill_content(url: str) -> str:
11834
"""Fetch SKILL.md content from an HTTPS URL.
11935
12036
Uses ``urllib.request`` (stdlib) so no additional dependencies are needed.
12137
12238
Args:
123-
url: The HTTPS URL to fetch.
39+
url: The HTTPS URL to fetch. Must point directly to the raw
40+
SKILL.md content (for example,
41+
``https://raw.githubusercontent.com/org/repo/main/SKILL.md``).
12442
12543
Returns:
12644
The response body as a string.
@@ -132,15 +50,14 @@ def fetch_skill_content(url: str) -> str:
13250
if not url.startswith("https://"):
13351
raise ValueError(f"url=<{url}> | only https:// URLs are supported")
13452

135-
resolved = resolve_to_raw_url(url)
136-
logger.info("url=<%s> | fetching skill content from %s", url, resolved)
53+
logger.info("url=<%s> | fetching skill content", url)
13754

13855
try:
139-
req = urllib.request.Request(resolved, headers={"User-Agent": "strands-agents-sdk"}) # noqa: S310
56+
req = urllib.request.Request(url, headers={"User-Agent": "strands-agents-sdk"}) # noqa: S310
14057
with urllib.request.urlopen(req, timeout=30) as response: # noqa: S310
14158
content: str = response.read().decode("utf-8")
14259
return content
14360
except urllib.error.HTTPError as e:
144-
raise RuntimeError(f"url=<{resolved}> | HTTP {e.code}: {e.reason}") from e
61+
raise RuntimeError(f"url=<{url}> | HTTP {e.code}: {e.reason}") from e
14562
except urllib.error.URLError as e:
146-
raise RuntimeError(f"url=<{resolved}> | failed to fetch skill: {e.reason}") from e
63+
raise RuntimeError(f"url=<{url}> | failed to fetch skill: {e.reason}") from e

src/strands/vended_plugins/skills/skill.py

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -337,26 +337,18 @@ def from_content(cls, content: str, *, strict: bool = False) -> Skill:
337337
def from_url(cls, url: str, *, strict: bool = False) -> Skill:
338338
"""Load a skill by fetching its SKILL.md content from an HTTPS URL.
339339
340-
Fetches the SKILL.md content over HTTPS and parses it using
341-
:meth:`from_content`. GitHub web URLs are automatically resolved
342-
to ``raw.githubusercontent.com``::
340+
Fetches the raw SKILL.md content over HTTPS and parses it using
341+
:meth:`from_content`. The URL must point directly to the raw
342+
file content (not an HTML page).
343343
344-
# Direct raw URL
345-
skill = Skill.from_url(
346-
"https://raw.githubusercontent.com/org/repo/main/SKILL.md"
347-
)
344+
Example::
348345
349-
# GitHub web URL (auto-resolved)
350346
skill = Skill.from_url(
351-
"https://github.com/org/repo/tree/main/skills/my-skill"
347+
"https://raw.githubusercontent.com/org/repo/main/SKILL.md"
352348
)
353349
354-
# Repository root with @ref
355-
skill = Skill.from_url("https://github.com/org/my-skill@v1.0.0")
356-
357350
Args:
358-
url: An ``https://`` URL pointing to a SKILL.md file, a GitHub
359-
repository, or a GitHub ``/tree/<ref>/path`` URL.
351+
url: An ``https://`` URL pointing directly to raw SKILL.md content.
360352
strict: If True, raise on any validation issue. If False (default),
361353
warn and load anyway.
362354

tests/strands/vended_plugins/skills/test_skill.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -571,24 +571,6 @@ def test_from_url_returns_skill(self):
571571
assert "Remote instructions." in skill.instructions
572572
assert skill.path is None
573573

574-
def test_from_url_github_web_url(self):
575-
"""Test that GitHub web URLs are handled (resolved in fetch_skill_content)."""
576-
from unittest.mock import patch
577-
578-
with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT):
579-
skill = Skill.from_url("https://github.com/org/repo/tree/main/skills/my-skill")
580-
581-
assert skill.name == "my-skill"
582-
583-
def test_from_url_with_ref(self):
584-
"""Test URL with @ref suffix."""
585-
from unittest.mock import patch
586-
587-
with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT):
588-
skill = Skill.from_url("https://github.com/org/my-skill@v1.0.0")
589-
590-
assert skill.name == "my-skill"
591-
592574
def test_from_url_invalid_url_raises(self):
593575
"""Test that a non-HTTPS URL raises ValueError."""
594576
with pytest.raises(ValueError, match="not a valid HTTPS URL"):

tests/strands/vended_plugins/skills/test_url_loader.py

Lines changed: 25 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -10,30 +10,27 @@
1010
from strands.vended_plugins.skills._url_loader import (
1111
fetch_skill_content,
1212
is_url,
13-
resolve_to_raw_url,
1413
)
1514

1615

1716
class TestIsUrl:
1817
"""Tests for is_url."""
1918

2019
def test_https_url(self):
21-
assert is_url("https://github.com/org/skill-repo") is True
20+
assert is_url("https://example.com/SKILL.md") is True
2221

23-
def test_https_raw_url(self):
22+
def test_https_raw_github_url(self):
2423
assert is_url("https://raw.githubusercontent.com/org/repo/main/SKILL.md") is True
2524

2625
def test_http_rejected(self):
2726
"""Plaintext http:// is rejected for security."""
28-
assert is_url("http://github.com/org/skill-repo") is False
27+
assert is_url("http://example.com/SKILL.md") is False
2928

3029
def test_ssh_rejected(self):
31-
"""SSH URLs are not supported in HTTPS-only mode."""
32-
assert is_url("ssh://git@github.com/org/skill-repo") is False
30+
assert is_url("ssh://git@github.com/org/repo") is False
3331

3432
def test_git_at_rejected(self):
35-
"""git@ URLs are not supported in HTTPS-only mode."""
36-
assert is_url("git@github.com:org/skill-repo.git") is False
33+
assert is_url("git@github.com:org/repo.git") is False
3734

3835
def test_local_relative_path(self):
3936
assert is_url("./skills/my-skill") is False
@@ -48,78 +45,6 @@ def test_empty_string(self):
4845
assert is_url("") is False
4946

5047

51-
class TestResolveToRawUrl:
52-
"""Tests for resolve_to_raw_url."""
53-
54-
def test_raw_url_passthrough(self):
55-
url = "https://raw.githubusercontent.com/org/repo/main/SKILL.md"
56-
assert resolve_to_raw_url(url) == url
57-
58-
def test_non_github_passthrough(self):
59-
url = "https://example.com/skills/SKILL.md"
60-
assert resolve_to_raw_url(url) == url
61-
62-
def test_repo_root(self):
63-
assert resolve_to_raw_url("https://github.com/org/repo") == (
64-
"https://raw.githubusercontent.com/org/repo/HEAD/SKILL.md"
65-
)
66-
67-
def test_repo_root_trailing_slash(self):
68-
assert resolve_to_raw_url("https://github.com/org/repo/") == (
69-
"https://raw.githubusercontent.com/org/repo/HEAD/SKILL.md"
70-
)
71-
72-
def test_repo_root_with_ref(self):
73-
assert resolve_to_raw_url("https://github.com/org/repo@v1.0.0") == (
74-
"https://raw.githubusercontent.com/org/repo/v1.0.0/SKILL.md"
75-
)
76-
77-
def test_repo_root_with_branch_ref(self):
78-
assert resolve_to_raw_url("https://github.com/org/repo@main") == (
79-
"https://raw.githubusercontent.com/org/repo/main/SKILL.md"
80-
)
81-
82-
def test_tree_url_directory(self):
83-
assert resolve_to_raw_url("https://github.com/org/repo/tree/main/skills/my-skill") == (
84-
"https://raw.githubusercontent.com/org/repo/main/skills/my-skill/SKILL.md"
85-
)
86-
87-
def test_tree_url_branch_only(self):
88-
assert resolve_to_raw_url("https://github.com/org/repo/tree/main") == (
89-
"https://raw.githubusercontent.com/org/repo/main/SKILL.md"
90-
)
91-
92-
def test_tree_url_trailing_slash(self):
93-
assert resolve_to_raw_url("https://github.com/org/repo/tree/main/skills/my-skill/") == (
94-
"https://raw.githubusercontent.com/org/repo/main/skills/my-skill/SKILL.md"
95-
)
96-
97-
def test_tree_url_with_tag(self):
98-
assert resolve_to_raw_url("https://github.com/org/repo/tree/v2.0/skills/brainstorming") == (
99-
"https://raw.githubusercontent.com/org/repo/v2.0/skills/brainstorming/SKILL.md"
100-
)
101-
102-
def test_blob_url_to_skill_md(self):
103-
assert resolve_to_raw_url("https://github.com/org/repo/blob/main/skills/my-skill/SKILL.md") == (
104-
"https://raw.githubusercontent.com/org/repo/main/skills/my-skill/SKILL.md"
105-
)
106-
107-
def test_blob_url_to_lowercase_skill_md(self):
108-
assert resolve_to_raw_url("https://github.com/org/repo/blob/main/skills/my-skill/skill.md") == (
109-
"https://raw.githubusercontent.com/org/repo/main/skills/my-skill/skill.md"
110-
)
111-
112-
def test_unrecognized_github_path_passthrough(self):
113-
"""Unrecognized GitHub URL patterns are returned as-is."""
114-
url = "https://github.com/org/repo/wiki/Some-Page"
115-
assert resolve_to_raw_url(url) == url
116-
117-
def test_github_deep_unrecognized_path(self):
118-
"""GitHub URLs with more than owner/repo segments (not tree/blob) pass through."""
119-
url = "https://github.com/org/repo/actions/runs/12345"
120-
assert resolve_to_raw_url(url) == url
121-
122-
12348
class TestFetchSkillContent:
12449
"""Tests for fetch_skill_content."""
12550

@@ -139,22 +64,33 @@ def test_fetch_success(self):
13964

14065
assert result == skill_content
14166

142-
def test_fetch_resolves_github_url(self):
143-
"""Test that GitHub web URLs are resolved before fetching."""
144-
skill_content = "---\nname: test\ndescription: test\n---\n"
67+
def test_fetch_uses_url_directly(self):
68+
"""Test that the URL is used as-is with no resolution."""
69+
url = "https://raw.githubusercontent.com/org/repo/main/skills/my-skill/SKILL.md"
14570

14671
mock_response = MagicMock()
147-
mock_response.read.return_value = skill_content.encode("utf-8")
72+
mock_response.read.return_value = b"---\nname: t\ndescription: t\n---\n"
73+
mock_response.__enter__ = MagicMock(return_value=mock_response)
74+
mock_response.__exit__ = MagicMock(return_value=False)
75+
76+
with patch(f"{self._LOADER}.urllib.request.urlopen", return_value=mock_response) as mock_urlopen:
77+
fetch_skill_content(url)
78+
79+
request_obj = mock_urlopen.call_args[0][0]
80+
assert request_obj.full_url == url
81+
82+
def test_fetch_sets_user_agent(self):
83+
"""Test that requests include a User-Agent header."""
84+
mock_response = MagicMock()
85+
mock_response.read.return_value = b"---\nname: t\ndescription: t\n---\n"
14886
mock_response.__enter__ = MagicMock(return_value=mock_response)
14987
mock_response.__exit__ = MagicMock(return_value=False)
15088

15189
with patch(f"{self._LOADER}.urllib.request.urlopen", return_value=mock_response) as mock_urlopen:
152-
fetch_skill_content("https://github.com/org/repo/tree/main/skills/my-skill")
90+
fetch_skill_content("https://example.com/SKILL.md")
15391

154-
# Verify the resolved raw URL was used
155-
call_args = mock_urlopen.call_args
156-
request_obj = call_args[0][0]
157-
assert "raw.githubusercontent.com" in request_obj.full_url
92+
request_obj = mock_urlopen.call_args[0][0]
93+
assert request_obj.get_header("User-agent") == "strands-agents-sdk"
15894

15995
def test_fetch_http_error(self):
16096
"""Test that HTTP errors raise RuntimeError."""

0 commit comments

Comments
 (0)