Skip to content

Commit 38ff8b7

Browse files
Simplify load_marketplace_skill_names - remove unused source field extraction
Address review feedback: - Remove load_marketplace_plugins function entirely - Simplify load_marketplace_skill_names to directly extract skill names - Update load_public_skills to use simplified interface - No longer extract source field that was immediately discarded This follows the "don't extract data you don't use" principle. Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 356a5ee commit 38ff8b7

File tree

3 files changed

+64
-36
lines changed

3 files changed

+64
-36
lines changed
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
#!/usr/bin/env python3
2+
"""Demonstration script for the marketplace_path feature.
3+
4+
This script demonstrates:
5+
1. Loading skills from the default marketplace
6+
2. Loading skills with marketplace_path=None (all skills)
7+
"""
8+
9+
import sys
10+
11+
sys.path.insert(0, "openhands-sdk")
12+
13+
from openhands.sdk.context.skills import load_public_skills
14+
15+
16+
def main():
17+
print("=" * 60)
18+
print("Marketplace Path Feature Demonstration")
19+
print("=" * 60)
20+
21+
# 1. Load from default marketplace
22+
print("\n1. Loading skills from default marketplace...")
23+
default_skills = load_public_skills()
24+
print(f" Loaded {len(default_skills)} skills from default marketplace:")
25+
for skill in sorted(default_skills, key=lambda s: s.name)[:5]:
26+
print(f" - {skill.name}")
27+
if len(default_skills) > 5:
28+
print(f" ... and {len(default_skills) - 5} more")
29+
30+
# 2. Load all skills (no marketplace filtering)
31+
print("\n2. Loading ALL skills (marketplace_path=None)...")
32+
all_skills = load_public_skills(marketplace_path=None)
33+
print(f" Loaded {len(all_skills)} skills without filtering:")
34+
for skill in sorted(all_skills, key=lambda s: s.name)[:5]:
35+
print(f" - {skill.name}")
36+
if len(all_skills) > 5:
37+
print(f" ... and {len(all_skills) - 5} more")
38+
39+
# Verify feature works
40+
print("\n" + "=" * 60)
41+
print("VERIFICATION:")
42+
if len(default_skills) > 0 and len(all_skills) >= len(default_skills):
43+
print("✓ Default marketplace loaded successfully")
44+
print("✓ All skills (no filter) loaded successfully")
45+
print("✓ marketplace_path parameter works correctly!")
46+
else:
47+
print("✗ Something went wrong")
48+
return 1
49+
print("=" * 60)
50+
return 0
51+
52+
53+
if __name__ == "__main__":
54+
sys.exit(main())

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

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ def load_project_skills(work_dir: str | Path) -> list[Skill]:
853853

854854
def load_marketplace_skill_names(
855855
repo_path: Path, marketplace_path: str
856-
) -> set[str] | None:
856+
) -> list[str] | None:
857857
"""Load the list of skill names from a marketplace manifest file.
858858
859859
Uses the existing Marketplace model from openhands.sdk.plugin to parse
@@ -864,30 +864,7 @@ def load_marketplace_skill_names(
864864
marketplace_path: Relative path to the marketplace JSON file within the repo.
865865
866866
Returns:
867-
Set of skill names to load, or None if marketplace file not found or invalid.
868-
"""
869-
result = load_marketplace_plugins(repo_path, marketplace_path)
870-
if result is None:
871-
return None
872-
return {name for name, _ in result}
873-
874-
875-
def load_marketplace_plugins(
876-
repo_path: Path, marketplace_path: str
877-
) -> list[tuple[str, str | None]] | None:
878-
"""Load plugin names and sources from a marketplace manifest file.
879-
880-
Uses the existing Marketplace model from openhands.sdk.plugin to parse
881-
the marketplace JSON file and extract plugin information.
882-
883-
Args:
884-
repo_path: Path to the local repository.
885-
marketplace_path: Relative path to the marketplace JSON file within the repo.
886-
887-
Returns:
888-
List of (skill_name, source) tuples where source is the plugin source string
889-
(e.g., "./skill-name" or "owner/repo:skills/skill-name"), or None if
890-
marketplace file not found or invalid.
867+
List of skill names to load, or None if marketplace file not found or invalid.
891868
"""
892869
from openhands.sdk.plugin import Marketplace
893870

@@ -903,15 +880,13 @@ def load_marketplace_plugins(
903880
# Use Marketplace model for validation and parsing
904881
marketplace = Marketplace.model_validate({**data, "path": str(repo_path)})
905882

906-
plugins = []
907-
for plugin in marketplace.plugins:
908-
source = plugin.source if isinstance(plugin.source, str) else None
909-
plugins.append((plugin.name, source))
883+
skill_names = [plugin.name for plugin in marketplace.plugins]
910884

911885
logger.debug(
912-
f"Loaded {len(plugins)} plugins from marketplace: {marketplace_path}"
886+
f"Loaded {len(skill_names)} skill names from marketplace: "
887+
f"{marketplace_path}"
913888
)
914-
return plugins
889+
return skill_names
915890

916891
except json.JSONDecodeError as e:
917892
logger.warning(f"Failed to parse marketplace JSON {marketplace_file}: {e}")
@@ -988,10 +963,9 @@ def load_public_skills(
988963
# Load the marketplace to determine which skills to include
989964
skill_names_to_load: list[str] | None = None
990965
if marketplace_path:
991-
marketplace_plugins = load_marketplace_plugins(repo_path, marketplace_path)
992-
if marketplace_plugins is not None:
993-
# Extract skill names from marketplace (ignore source field)
994-
skill_names_to_load = [name for name, _ in marketplace_plugins]
966+
skill_names_to_load = load_marketplace_skill_names(
967+
repo_path, marketplace_path
968+
)
995969

996970
# Find the skills directory
997971
skills_dir = repo_path / "skills"

tests/sdk/context/skill/test_load_public_skills.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ def test_load_marketplace_skill_names_returns_skill_names(mock_repo_with_marketp
641641
)
642642

643643
assert skill_names is not None
644-
assert skill_names == {"git", "docker"}
644+
assert set(skill_names) == {"git", "docker"}
645645

646646

647647
def test_load_marketplace_skill_names_returns_none_when_file_missing(tmp_path):

0 commit comments

Comments
 (0)