skills hub: don't abort installs on missing referenced support files
A SKILL.md that mentions a support-path token which doesn't exist in the repo (a prose glob like `references/type-*.md`, a placeholder truncated by the regex, or a repo-only dev script) previously made GitHubSource and UrlSource fetch fail with 'Could not fetch ... from any source', even though every real file was present. - _referenced_support_paths: skip glob/placeholder tokens and bare prefix matches that can't name an actual file. - GitHubSource/UrlSource fetch: warn and continue when a referenced support file is missing or unfetchable, bundling what exists, instead of aborting the whole install. Non-regular tree entries (e.g. git symlinks) are still rejected. Regression tests for both syntax filtering and the fetch loops.
This commit is contained in:
@@ -1918,3 +1918,70 @@ class TestLoadHermesIndex:
|
||||
|
||||
data = hub._load_hermes_index()
|
||||
assert data == {"skills": [{"name": "stale"}]}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Referenced-path extraction & missing support files (regression: a prose
|
||||
# glob or a repo-only dev tool referenced in SKILL.md must not abort install)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestReferencedSupportPaths:
|
||||
def test_ignores_globs_placeholders_and_truncated_tokens(self):
|
||||
md = (
|
||||
"Load `references/type-*.md` before drawing, and "
|
||||
"`references/type-<name>.md` for the matching type.\n"
|
||||
"Real files: [flowchart](references/type-flowchart.md) and "
|
||||
"`references/type-architecture.md`.\n"
|
||||
)
|
||||
assert _referenced_support_paths(md) == {
|
||||
"references/type-flowchart.md",
|
||||
"references/type-architecture.md",
|
||||
}
|
||||
|
||||
def test_keeps_real_backtick_references(self):
|
||||
md = "Run `python3 scripts/self_check.py <file>` and see `references/guide.md`.\n"
|
||||
assert _referenced_support_paths(md) == {
|
||||
"scripts/self_check.py",
|
||||
"references/guide.md",
|
||||
}
|
||||
|
||||
|
||||
class TestGitHubSourceFetchMissingReferencedFile:
|
||||
def _source(self):
|
||||
auth = MagicMock(spec=GitHubAuth)
|
||||
return GitHubSource(auth=auth)
|
||||
|
||||
def test_fetch_skips_missing_referenced_file_and_keeps_the_rest(self):
|
||||
md = (
|
||||
"---\nname: demo\ndescription: demo\n---\n\n"
|
||||
"See [guide](references/guide.md) and `references/missing.md`.\n"
|
||||
)
|
||||
tree_entries = [
|
||||
{"path": "skills/demo/references/guide.md", "type": "blob", "mode": "100644"},
|
||||
]
|
||||
source = self._source()
|
||||
with patch.object(source, "_fetch_file_content", return_value=md), \
|
||||
patch.object(source, "_get_repo_tree", return_value=("main", tree_entries)), \
|
||||
patch.object(source, "_fetch_file_bytes", return_value=b"# guide"):
|
||||
bundle = source.fetch("owner/repo/skills/demo")
|
||||
|
||||
assert bundle is not None
|
||||
assert bundle.name == "demo"
|
||||
# The present referenced file is bundled…
|
||||
assert bundle.files["references/guide.md"] == b"# guide"
|
||||
# …and the missing one is warned about and skipped, not fatal.
|
||||
assert "references/missing.md" not in bundle.files
|
||||
|
||||
|
||||
class TestUrlSourceFetchMissingReferencedFile:
|
||||
def test_fetch_skips_missing_referenced_file(self):
|
||||
md = "---\nname: demo\ndescription: demo\n---\n\nSee `references/missing.md`.\n"
|
||||
source = UrlSource()
|
||||
with patch.object(source, "_fetch_text", return_value=md), \
|
||||
patch.object(source, "_fetch_bytes", return_value=None):
|
||||
bundle = source.fetch("https://example.com/skills/demo/SKILL.md")
|
||||
|
||||
assert bundle is not None
|
||||
assert bundle.name == "demo"
|
||||
assert "references/missing.md" not in bundle.files
|
||||
|
||||
+12
-2
@@ -209,6 +209,12 @@ def _referenced_support_paths(skill_md: str) -> Optional[set[str]]:
|
||||
except ValueError:
|
||||
return None
|
||||
if safe.split("/", 1)[0] in _ALLOWED_SUPPORT_DIRS:
|
||||
# Prose globs/placeholders — e.g. ``references/type-*.md`` or
|
||||
# ``references/type-<name>.md`` (which the regex truncates to the
|
||||
# bare prefix ``references/type-``) — are agent instructions, not
|
||||
# files. Only tokens that can name an actual file are references.
|
||||
if re.search(r"[*?<>]", safe) or "." not in safe.rsplit("/", 1)[-1]:
|
||||
continue
|
||||
paths.add(safe)
|
||||
return paths
|
||||
|
||||
@@ -733,7 +739,9 @@ class GitHubSource(SkillSource):
|
||||
return None
|
||||
content = self._fetch_file_bytes(repo, item_path)
|
||||
if content is None:
|
||||
return None
|
||||
logger.warning("Failed to fetch referenced skill support "
|
||||
"file; continuing without it: %s", item_path)
|
||||
continue
|
||||
files[rel_path] = content
|
||||
# A support file SKILL.md links must actually exist as a regular
|
||||
# file — a missing or symlinked referenced path rejects the
|
||||
@@ -750,7 +758,9 @@ class GitHubSource(SkillSource):
|
||||
for rel_path in referenced:
|
||||
content = self._fetch_file_bytes(repo, f"{skill_path.rstrip('/')}/{rel_path}")
|
||||
if content is None:
|
||||
return None
|
||||
logger.warning("Failed to fetch referenced skill support "
|
||||
"file; continuing without it: %s", rel_path)
|
||||
continue
|
||||
files[rel_path] = content
|
||||
revision = ""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user