fix downloading skills

Author: alcholiclg

ms_agent/skill/catalog.py

@@ -1,10 +1,14 @@
 # Copyright (c) ModelScope Contributors. All rights reserved.
 import os
+import shutil
 import subprocess
 import tempfile
+import zipfile
 from pathlib import Path
 from typing import Dict, List, Optional, Set
 
+import requests
+
 from ms_agent.utils.logger import get_logger
 
 from .loader import SkillLoader
@@ -13,6 +17,55 @@
 
 logger = get_logger()
 
+MODELSCOPE_SKILL_API = (
+    "https://www.modelscope.cn/api/v1/skills/{skill_id}/archive/zip/master")
+
+
+def _download_skill_zip(skill_id: str, local_dir: str) -> str:
+    """Download a skill archive from the ModelScope skill hub and extract it.
+
+    This is a pure-HTTP fallback that does not require ``modelscope>=1.35.2``.
+    The directory naming follows the SDK convention: ``<element_name>``.
+    """
+    url = MODELSCOPE_SKILL_API.format(skill_id=skill_id)
+    os.makedirs(local_dir, exist_ok=True)
+
+    _owner, name = skill_id.split("/", 1)
+    skill_dir = os.path.join(local_dir, name)
+
+    resp = requests.get(url, stream=True, timeout=120)
+    resp.raise_for_status()
+
+    zip_path = os.path.join(local_dir, f"{name}.zip")
+    try:
+        with open(zip_path, "wb") as fh:
+            for chunk in resp.iter_content(chunk_size=8192):
+                if chunk:
+                    fh.write(chunk)
+
+        if os.path.exists(skill_dir):
+            shutil.rmtree(skill_dir)
+        os.makedirs(skill_dir, exist_ok=True)
+
+        with zipfile.ZipFile(zip_path, "r") as zf:
+            zf.extractall(skill_dir)
+
+        entries = os.listdir(skill_dir)
+        if len(entries) == 1:
+            nested = os.path.join(skill_dir, entries[0])
+            if os.path.isdir(nested):
+                for item in os.listdir(nested):
+                    shutil.move(
+                        os.path.join(nested, item),
+                        os.path.join(skill_dir, item))
+                os.rmdir(nested)
+    finally:
+        if os.path.exists(zip_path):
+            os.remove(zip_path)
+
+    logger.info(f"Skill {skill_id} downloaded to {skill_dir}")
+    return skill_dir
+
 BUILTIN_SKILLS_DIR = Path(__file__).parent.parent / "skills"
 if not BUILTIN_SKILLS_DIR.exists():
     _repo_root = Path(__file__).parent.parent.parent
@@ -129,10 +182,16 @@ def _materialize_and_load(
 
     def _load_from_modelscope(
             self, source: SkillSource) -> Dict[str, SkillSchema]:
-        from modelscope import snapshot_download
-        local_path = snapshot_download(
-            repo_id=source.repo_id,
-            revision=source.revision or "master")
+        try:
+            from modelscope.hub.api import HubApi
+            api = HubApi()
+            local_dir = str(USER_SKILLS_DIR / "installed")
+            local_path = api.download_skill(
+                skill_id=source.repo_id, local_dir=local_dir)
+        except (ImportError, AttributeError):
+            local_path = _download_skill_zip(
+                source.repo_id,
+                str(USER_SKILLS_DIR / "installed"))
         if source.subdir:
             local_path = str(Path(local_path) / source.subdir)
         return self._loader.load_skills(local_path)

ms_agent/skill/loader.py

@@ -41,12 +41,7 @@ def load_skills(
             logger.warning('No skills provided to load.')
             return all_skills
 
-        def is_skill_id(s: str) -> bool:
-            return '/' in s and len(s.split('/')) == 2 and all(
-                s.split('/')) and not os.path.exists(s)
-
         if isinstance(skills, str):
-            # Could be a single skill path, root path of skills, or skill ID on ModelScope hub
             skill_list = [skills]
         elif all(isinstance(s, str) for s in skills) or all(
                 isinstance(s, SkillSchema) for s in skills):
@@ -55,12 +50,6 @@ def is_skill_id(s: str) -> bool:
             raise ValueError('Invalid skills input type.')
 
         for skill in skill_list:
-
-            if is_skill_id(skill):
-                from modelscope import snapshot_download
-                skill_path: str = snapshot_download(repo_id=skill)
-                skill = skill_path
-
             if isinstance(skill, SkillSchema):
                 skill_key = self._get_skill_key(skill=skill)
                 all_skills[skill_key] = skill

ms_agent/skill/sources.py

@@ -26,21 +26,40 @@ class SkillSource:
 
 _MODELSCOPE_URI_RE = re.compile(
     r'^modelscope://(?P<repo>[^@#]+)(?:@(?P<rev>[^#]+))?(?:#(?P<sub>.+))?$')
+
+_MODELSCOPE_SKILL_URL_RE = re.compile(
+    r'^https?://(?:www\.)?modelscope\.(?:cn|ai)/skills/'
+    r'(?P<repo>[^/]+/[^/]+)(?:/.*)?$')
+
 _OWNER_REPO_RE = re.compile(r'^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$')
 
+_AT_PREFIX_RE = re.compile(
+    r'^@(?P<repo>[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)$')
+
+
+def _looks_like_path(raw: str) -> bool:
+    """Return True when *raw* is clearly meant to be a local filesystem path
+    rather than a hub identifier (e.g. starts with ``/``, ``./``, ``~``, or
+    contains path separators that don't match the ``owner/repo`` pattern).
+    """
+    return raw.startswith(('/', './', '../', '~'))
+
 
 def parse_skill_source(raw: str) -> SkillSource:
     """Parse a raw string into a SkillSource.
 
-    Supported formats:
-      - /abs/path/to/skills           -> LOCAL_DIR
-      - ./relative/path               -> LOCAL_DIR
-      - modelscope://owner/repo@rev   -> MODELSCOPE
-      - https://... or git://...      -> GIT
-      - owner/repo                    -> MODELSCOPE (when path does not exist)
+    Supported formats (checked in order):
+      - /abs/path  or  ./rel/path  or  ~/path     -> LOCAL_DIR
+      - modelscope://owner/repo[@rev][#subdir]     -> MODELSCOPE
+      - https://modelscope.cn/skills/owner/repo    -> MODELSCOPE
+      - @owner/repo  (CLI shorthand)               -> MODELSCOPE
+      - https://... or git://...                   -> GIT
+      - owner/repo  (when path does not exist)     -> MODELSCOPE
+      - anything else                              -> LOCAL_DIR
     """
-    if os.path.exists(raw):
-        return SkillSource(type=SkillSourceType.LOCAL_DIR, path=raw)
+    if _looks_like_path(raw):
+        resolved = str(Path(raw).expanduser().resolve())
+        return SkillSource(type=SkillSourceType.LOCAL_DIR, path=resolved)
 
     m = _MODELSCOPE_URI_RE.match(raw)
     if m:
@@ -51,10 +70,25 @@ def parse_skill_source(raw: str) -> SkillSource:
             subdir=m.group('sub'),
         )
 
+    m = _MODELSCOPE_SKILL_URL_RE.match(raw)
+    if m:
+        return SkillSource(
+            type=SkillSourceType.MODELSCOPE,
+            repo_id=m.group('repo'),
+        )
+
+    m = _AT_PREFIX_RE.match(raw)
+    if m:
+        return SkillSource(
+            type=SkillSourceType.MODELSCOPE,
+            repo_id=m.group('repo'),
+        )
+
     if raw.startswith(('https://', 'http://', 'git://')):
         return SkillSource(type=SkillSourceType.GIT, url=raw)
 
-    if _OWNER_REPO_RE.match(raw):
+    if _OWNER_REPO_RE.match(raw) and not os.path.exists(raw):
         return SkillSource(type=SkillSourceType.MODELSCOPE, repo_id=raw)
 
-    return SkillSource(type=SkillSourceType.LOCAL_DIR, path=raw)
+    resolved = str(Path(raw).resolve()) if not os.path.isabs(raw) else raw
+    return SkillSource(type=SkillSourceType.LOCAL_DIR, path=resolved)

requirements/framework.txt

@@ -6,7 +6,7 @@ json5
 markdown
 matplotlib
 mcp
-modelscope
+modelscope>=1.35.2
 moviepy
 numpy
 omegaconf