Skip to content

Commit 415723a

Browse files
refactor: remove dead load_marketplace_skill_names, log Plugin.load failures
load_marketplace_skill_names lost its only production caller when the previous commit switched load_public_skills to use _load_marketplace_object directly. Remove it and its 4 tests. Replace the silent 'except Exception: pass' in the remote fetch path with a logger.debug so Plugin.load failures are visible when debugging. Co-authored-by: openhands <openhands@all-hands.dev>
1 parent ca89b9a commit 415723a

File tree

2 files changed

+6
-72
lines changed

2 files changed

+6
-72
lines changed

openhands-sdk/openhands/sdk/context/skills/skill.py

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -900,32 +900,6 @@ def load_project_skills(work_dir: str | Path) -> list[Skill]:
900900
DEFAULT_MARKETPLACE_PATH = "marketplaces/default.json"
901901

902902

903-
def load_marketplace_skill_names(
904-
repo_path: Path, marketplace_path: str
905-
) -> set[str] | None:
906-
"""Load the list of skill names from a marketplace manifest file.
907-
908-
Uses the existing Marketplace model from openhands.sdk.plugin to parse
909-
the marketplace JSON file and extract plugin names.
910-
911-
Args:
912-
repo_path: Path to the local repository.
913-
marketplace_path: Relative path to the marketplace JSON file within the repo.
914-
915-
Returns:
916-
Set of skill names to load, or None if marketplace file not found or invalid.
917-
"""
918-
marketplace = _load_marketplace_object(repo_path, marketplace_path)
919-
if marketplace is None:
920-
return None
921-
922-
skill_names = {plugin.name for plugin in marketplace.plugins}
923-
logger.debug(
924-
f"Loaded {len(skill_names)} skill names from marketplace: {marketplace_path}"
925-
)
926-
return skill_names
927-
928-
929903
def _load_marketplace_object(
930904
repo_path: Path, marketplace_path: str
931905
) -> "Marketplace | None":
@@ -1045,8 +1019,12 @@ def _resolve_plugin_skills(
10451019
skills = plugin.get_all_skills()
10461020
if skills:
10471021
return skills
1048-
except Exception:
1049-
pass
1022+
except Exception as e:
1023+
logger.debug(
1024+
"Plugin.load failed for '%s', trying raw directory: %s",
1025+
plugin_entry.name,
1026+
e,
1027+
)
10501028
return _load_skills_from_resolved_path(plugin_path)
10511029

10521030
# Local source: try to load from the resolved path

tests/sdk/context/skill/test_load_public_skills.py

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
Skill,
1313
load_public_skills,
1414
)
15-
from openhands.sdk.context.skills.skill import load_marketplace_skill_names
1615
from openhands.sdk.context.skills.utils import update_skills_repository
1716

1817

@@ -674,49 +673,6 @@ def mock_repo_with_marketplace(tmp_path):
674673
return repo_dir
675674

676675

677-
def test_load_marketplace_skill_names_returns_skill_names(mock_repo_with_marketplace):
678-
"""Test that load_marketplace_skill_names correctly extracts skill names."""
679-
skill_names = load_marketplace_skill_names(
680-
mock_repo_with_marketplace, "marketplaces/default.json"
681-
)
682-
683-
assert skill_names is not None
684-
assert skill_names == {"git", "docker"}
685-
686-
687-
def test_load_marketplace_skill_names_returns_none_when_file_missing(tmp_path):
688-
"""Test that load_marketplace_skill_names returns None when file doesn't exist."""
689-
repo_dir = tmp_path / "repo"
690-
repo_dir.mkdir()
691-
692-
result = load_marketplace_skill_names(repo_dir, "marketplaces/default.json")
693-
assert result is None
694-
695-
696-
def test_load_marketplace_skill_names_returns_none_for_invalid_json(tmp_path):
697-
"""Test that load_marketplace_skill_names handles invalid JSON gracefully."""
698-
repo_dir = tmp_path / "repo"
699-
repo_dir.mkdir()
700-
marketplaces_dir = repo_dir / "marketplaces"
701-
marketplaces_dir.mkdir()
702-
(marketplaces_dir / "default.json").write_text("{ invalid json }")
703-
704-
result = load_marketplace_skill_names(repo_dir, "marketplaces/default.json")
705-
assert result is None
706-
707-
708-
def test_load_marketplace_skill_names_returns_none_for_missing_plugins(tmp_path):
709-
"""Test that load_marketplace_skill_names handles missing plugins key."""
710-
repo_dir = tmp_path / "repo"
711-
repo_dir.mkdir()
712-
marketplaces_dir = repo_dir / "marketplaces"
713-
marketplaces_dir.mkdir()
714-
(marketplaces_dir / "default.json").write_text(json.dumps({"name": "test"}))
715-
716-
result = load_marketplace_skill_names(repo_dir, "marketplaces/default.json")
717-
assert result is None
718-
719-
720676
def test_load_public_skills_filters_by_marketplace(
721677
mock_repo_with_marketplace, tmp_path
722678
):

0 commit comments

Comments
 (0)