From 4838ba46a4fd053b6bca2b21f150829c4f2d9eda Mon Sep 17 00:00:00 2001 From: alcholiclg Date: Tue, 1 Sep 2026 16:28:20 +0800 Subject: [PATCH 1/4] Optimize skill discovery and runtime synchronization --- ms_agent/skill/__init__.py | 2 + ms_agent/skill/discovery.py | 29 +++++++ ms_agent/skill/loader.py | 127 +++++++++++++++++++++++----- ms_agent/skill/runtime.py | 8 +- ms_agent/skill/schema.py | 79 ++++++++++++++--- tests/skill/test_live_tree.py | 21 +++++ tests/skill/test_skill_discovery.py | 112 ++++++++++++++++++++++++ 7 files changed, 344 insertions(+), 34 deletions(-) create mode 100644 ms_agent/skill/discovery.py create mode 100644 tests/skill/test_skill_discovery.py diff --git a/ms_agent/skill/__init__.py b/ms_agent/skill/__init__.py index dea0ba194..3894a820e 100644 --- a/ms_agent/skill/__init__.py +++ b/ms_agent/skill/__init__.py @@ -1,5 +1,6 @@ # Copyright (c) ModelScope Contributors. All rights reserved. from .catalog import SkillCatalog +from .discovery import SkillDescriptor from .loader import SkillLoader, load_skills from .prompt_injector import SkillPromptInjector from .safety import SafetyFinding, SkillSafetyReport, SkillSafetyScanner @@ -12,6 +13,7 @@ 'SkillSchema', 'SkillSchemaParser', 'SkillFile', + 'SkillDescriptor', 'SkillLoader', 'load_skills', 'SkillSource', diff --git a/ms_agent/skill/discovery.py b/ms_agent/skill/discovery.py new file mode 100644 index 000000000..0d4be77af --- /dev/null +++ b/ms_agent/skill/discovery.py @@ -0,0 +1,29 @@ +# Copyright (c) ModelScope Contributors. All rights reserved. +"""Lightweight discovery primitives for local Agent Skills. + +Discovery intentionally reads only ``SKILL.md``. Full file materialization, +safety scanning, and registration remain the catalog's responsibility. +""" +from __future__ import annotations + +from dataclasses import dataclass, field +from pathlib import Path +from typing import Optional + + +@dataclass(frozen=True) +class SkillDescriptor: + """Metadata discovered from a skill root without walking support files.""" + + skill_id: str + name: str + description: str + content: str + version: str = 'latest' + author: Optional[str] = None + tags: tuple[str, ...] = field(default_factory=tuple) + skill_path: Path = field(default_factory=lambda: Path.cwd().resolve()) + + @property + def key(self) -> str: + return f'{self.skill_id}@{self.version}' diff --git a/ms_agent/skill/loader.py b/ms_agent/skill/loader.py index 2f62e6383..9d3c8b244 100644 --- a/ms_agent/skill/loader.py +++ b/ms_agent/skill/loader.py @@ -4,9 +4,10 @@ import os import re from pathlib import Path -from typing import Dict, List, Optional, Union +from typing import Dict, Iterable, List, Optional, Union from ms_agent.utils.logger import logger +from .discovery import SkillDescriptor from .schema import SkillSchema, SkillSchemaParser @@ -22,6 +23,9 @@ class SkillLoader: def __init__(self): self.loaded_skills: Dict[str, SkillSchema] = {} self.parser = SkillSchemaParser() + self._discovery_cache: Dict[Path, + tuple[tuple[int, int, int], + Optional[SkillDescriptor]]] = {} def load_skills( self, skills: Union[str, List[str], List[SkillSchema]] @@ -80,6 +84,48 @@ def load_skills( return all_skills + def discover_skills( + self, skills: Union[str, List[str]]) -> Dict[str, SkillDescriptor]: + """Discover local skills without traversing their support files. + + The source ordering, hidden-directory rule, maximum nesting depth, + and "skill root is a leaf" rule match :meth:`load_skills`. Only + ``SKILL.md`` is read; callers that execute a skill must still use the + full catalog loading path. + """ + if not skills: + return {} + if isinstance(skills, str): + skill_list = [skills] + elif all(isinstance(skill, str) for skill in skills): + skill_list = skills + else: + raise ValueError('Invalid skills input type.') + + discovered: Dict[str, SkillDescriptor] = {} + active_roots: set[Path] = set() + for value in skill_list: + path = Path(value) + if not path.exists(): + logger.warning(f'Path does not exist: {path} - Skipping.') + continue + roots: Iterable[Path] + if self._is_skill_directory(path): + roots = (path, ) + else: + roots = self._iter_skill_directories(path) + for root in roots: + active_roots.add(root.resolve()) + descriptor = self._discover_single_skill(root) + if descriptor is not None: + discovered[descriptor.key] = descriptor + self._discovery_cache = { + path: cached + for path, cached in self._discovery_cache.items() + if path in active_roots + } + return discovered + def _is_skill_directory(self, path: Path) -> bool: """ Check if a directory is a valid skill directory. @@ -122,10 +168,65 @@ def _load_single_skill(self, skill_dir: Path) -> Optional[SkillSchema]: logger.error(f'Error loading skill ({skill_dir}): {str(e)}') return None + def _discover_single_skill(self, + skill_dir: Path) -> Optional[SkillDescriptor]: + try: + skill_dir = skill_dir.resolve() + skill_md = skill_dir / 'SKILL.md' + file_stat = skill_md.stat() + fingerprint = (file_stat.st_mtime_ns, file_stat.st_ctime_ns, + file_stat.st_size) + cached = self._discovery_cache.get(skill_dir) + if cached is not None and cached[0] == fingerprint: + return cached[1] + + content = skill_md.read_text(encoding='utf-8') + frontmatter = self.parser.parse_yaml_frontmatter(content) + if (not frontmatter or 'name' not in frontmatter + or 'description' not in frontmatter): + self._discovery_cache[skill_dir] = (fingerprint, None) + return None + descriptor = SkillDescriptor( + skill_id=skill_dir.name, + name=frontmatter['name'], + description=frontmatter['description'], + content=content, + version=frontmatter.get('version', 'latest'), + author=frontmatter.get('author'), + tags=tuple(frontmatter.get('tags') or []), + skill_path=skill_dir, + ) + self._discovery_cache[skill_dir] = (fingerprint, descriptor) + return descriptor + except Exception as exc: + logger.error(f'Error discovering skill ({skill_dir}): {exc}') + return None + #: How deep _scan_and_load_skills descends below the scan root. Bounds #: symlink cycles; deep enough for organizational nesting (category dirs). _MAX_SCAN_DEPTH = 5 + def _iter_skill_directories(self, base_path: Path) -> Iterable[Path]: + """Yield skill roots using the same bounded marker walk as loading.""" + if not base_path.is_dir(): + logger.warning(f'Not a valid directory: {base_path}') + return + + def _walk(directory: Path, depth: int) -> Iterable[Path]: + try: + entries = sorted(directory.iterdir()) + except OSError: + return + for item in entries: + if not item.is_dir() or item.name.startswith('.'): + continue + if self._is_skill_directory(item): + yield item + elif depth < self._MAX_SCAN_DEPTH: + yield from _walk(item, depth + 1) + + yield from _walk(base_path, 1) + def _scan_and_load_skills(self, base_path: Path) -> Dict[str, SkillSchema]: """ Recursively scan a tree and load every skill root found. @@ -144,26 +245,10 @@ def _scan_and_load_skills(self, base_path: Path) -> Dict[str, SkillSchema]: """ skills: Dict[str, SkillSchema] = {} - if not base_path.is_dir(): - logger.warning(f'Not a valid directory: {base_path}') - return skills - - def _walk(directory: Path, depth: int) -> None: - try: - entries = sorted(directory.iterdir()) - except OSError: - return - for item in entries: - if not item.is_dir() or item.name.startswith('.'): - continue - if self._is_skill_directory(item): - skill = self._load_single_skill(item) - if skill: - skills[self._get_skill_key(skill=skill)] = skill - elif depth < self._MAX_SCAN_DEPTH: - _walk(item, depth + 1) - - _walk(base_path, 1) + for skill_dir in self._iter_skill_directories(base_path): + skill = self._load_single_skill(skill_dir) + if skill: + skills[self._get_skill_key(skill=skill)] = skill return skills @staticmethod diff --git a/ms_agent/skill/runtime.py b/ms_agent/skill/runtime.py index 42b348f71..12905e59e 100644 --- a/ms_agent/skill/runtime.py +++ b/ms_agent/skill/runtime.py @@ -182,14 +182,18 @@ def reload_all(self) -> None: self._catalog.reload() self._version += 1 - def sync_with_config(self, skills_config) -> bool: + def sync_with_config(self, skills_config, *, force: bool = True) -> bool: """Resync the catalog from an updated skills config; bump the version only when the effective skill surface (inventory, metadata, disabled set) actually changed, so maybe_refresh_system_prompt() rebuilds messages[0] on real change and stays a no-op otherwise. - Returns True when the surface changed. + Existing callers retain the full-resync behavior by default. A host + with an authoritative change tracker may pass ``force=False`` to skip + I/O for a known-clean turn. Returns True when the surface changed. """ + if not force: + return False before = self._surface() self._catalog.resync(skills_config) after = self._surface() diff --git a/ms_agent/skill/schema.py b/ms_agent/skill/schema.py index 4b7ae76ea..bcdc0094c 100644 --- a/ms_agent/skill/schema.py +++ b/ms_agent/skill/schema.py @@ -5,7 +5,10 @@ Defines the data structure and validation logic for Agent Skills. Each Skill is represented as a self-contained directory with metadata. """ +import hashlib +import os import re +import stat import yaml from dataclasses import dataclass, field from pathlib import Path @@ -233,13 +236,17 @@ def is_ignored_path(p: Path) -> bool: Returns: True if path should be ignored, False otherwise """ + return SkillSchemaParser._is_ignored_name(p.name) + + @staticmethod + def _is_ignored_name(name: str) -> bool: ignored_names = { '.DS_Store', '__pycache__', '.git', '.gitignore', '.pytest_cache', '.mypy_cache' } ignored_suffixes = {'.pyc', '.pyo'} - - return (p.name in ignored_names) or (p.suffix in ignored_suffixes) + return ((name in ignored_names) + or (os.path.splitext(name)[1] in ignored_suffixes)) @staticmethod def parse_skill_directory(directory_path: Path) -> Optional[SkillSchema]: @@ -276,31 +283,77 @@ def parse_skill_directory(directory_path: Path) -> Optional[SkillSchema]: scripts = [] references = [] resources = [] + digest = hashlib.sha256() - for file_path in directory_path.rglob('*'): - if file_path.is_file(): - if SkillSchemaParser.is_ignored_path(file_path): + def _walk(current_path: Path, relative_dir: str, + visible_dir: bool) -> None: + # DirEntry keeps type/stat data from the same scandir call. This + # avoids a second filesystem lookup while retaining deterministic + # files-first DFS order (the legacy notice signature's order). + try: + with os.scandir(current_path) as iterator: + entries = sorted(iterator, key=lambda entry: entry.name) + except OSError: + return + directories = [] + for entry in entries: + try: + if entry.is_dir(follow_symlinks=False): + directories.append(entry.name) + continue + file_stat = entry.stat(follow_symlinks=True) + except OSError: + continue + if not stat.S_ISREG(file_stat.st_mode): continue - file_type = file_path.suffix if file_path.suffix else '.unknown' + filename = entry.name + file_path = current_path / filename + relative = ( + filename + if not relative_dir else f'{relative_dir}/{filename}') + if visible_dir and not filename.startswith('.'): + signature_row = (f'{relative}|{file_stat.st_mtime_ns}|' + f'{file_stat.st_size}\n') + digest.update(signature_row.encode()) + + if SkillSchemaParser._is_ignored_name(filename): + continue + suffix = os.path.splitext(filename)[1] + file_type = suffix if suffix else '.unknown' skill_file = SkillFile( - name=file_path.name, + name=filename, type=file_type, path=file_path, - required=(file_path.name == 'SKILL.md')) + required=(filename == 'SKILL.md')) files.append(skill_file) # Get scripts, references and resources if skill_file.type in SUPPORTED_SCRIPT_EXT: scripts.append(skill_file) - elif skill_file.type in ['.md' - ] and skill_file.name != 'SKILL.md': + elif (skill_file.type == '.md' + and skill_file.name != 'SKILL.md'): references.append(skill_file) else: resources.append(skill_file) - return SkillSchema( + # Hidden dirs remain in full materialization because the parser + # historically included their non-ignored support files. Only the + # private change signature prunes them. + for dirname in directories: + child_relative = ( + dirname + if not relative_dir else f'{relative_dir}/{dirname}') + _walk( + current_path / dirname, + child_relative, + visible_dir and not dirname.startswith('.'), + ) + + _walk(directory_path, '', True) + + schema = SkillSchema( skill_id=skill_id, name=frontmatter['name'], description=frontmatter['description'], @@ -314,6 +367,10 @@ def parse_skill_directory(directory_path: Path) -> Optional[SkillSchema]: references=references, resources=resources, ) + # Private, in-memory-only bridge for hosts that already had to perform + # this full traversal. It deliberately does not alter serialization. + schema._files_signature = digest.hexdigest()[:16] + return schema @staticmethod def validate_skill_schema(schema: SkillSchema) -> List[str]: diff --git a/tests/skill/test_live_tree.py b/tests/skill/test_live_tree.py index b7b7f649b..303d83042 100644 --- a/tests/skill/test_live_tree.py +++ b/tests/skill/test_live_tree.py @@ -198,6 +198,27 @@ def test_duplicate_sources_load_once(self, tmp_path): class TestRuntimeSync: + def test_known_clean_sync_skips_catalog_io(self, tmp_path, monkeypatch): + s1 = _mk_skill(tmp_path, 'alpha') + cfg = _skills_config([s1]) + cat = SkillCatalog(config=cfg) + cat.load_from_config(cfg) + rt = SkillRuntime(catalog=cat) + calls = [] + original_resync = cat.resync + + def _resync(value): + calls.append(value) + return original_resync(value) + + monkeypatch.setattr(cat, 'resync', _resync) + + assert rt.sync_with_config(cfg, force=False) is False + assert calls == [] + + assert rt.sync_with_config(cfg) is False + assert calls == [cfg] + def test_sync_bumps_version_only_on_change(self, tmp_path): s1 = _mk_skill(tmp_path, 'alpha') cfg = _skills_config([s1]) diff --git a/tests/skill/test_skill_discovery.py b/tests/skill/test_skill_discovery.py new file mode 100644 index 000000000..9e0f7477e --- /dev/null +++ b/tests/skill/test_skill_discovery.py @@ -0,0 +1,112 @@ +import hashlib +import os +from pathlib import Path + +from ms_agent.skill.loader import SkillLoader +from ms_agent.skill.schema import SkillSchemaParser + + +def _make_skill(root: Path, name: str, description: str = 'description'): + skill = root / name + skill.mkdir(parents=True) + (skill / 'SKILL.md').write_text( + f'---\nname: {name}\ndescription: {description}\n---\nbody', + encoding='utf-8', + ) + return skill + + +def _legacy_signature(root: Path) -> str: + digest = hashlib.sha256() + for current, dirs, files in os.walk(root): + dirs[:] = sorted(name for name in dirs if not name.startswith('.')) + for name in sorted(name for name in files if not name.startswith('.')): + path = Path(current) / name + try: + file_stat = path.stat() + except OSError: + continue + relative = path.relative_to(root).as_posix() + digest.update( + f'{relative}|{file_stat.st_mtime_ns}|{file_stat.st_size}\n'. + encode()) + return digest.hexdigest()[:16] + + +def test_discovery_reads_metadata_without_support_tree_walk( + tmp_path, monkeypatch): + skill = _make_skill(tmp_path, 'alpha') + (skill / 'references').mkdir() + (skill / 'references' / 'large.md').write_text('payload', encoding='utf-8') + + def _unexpected_rglob(*args, **kwargs): + raise AssertionError( + 'metadata discovery must not traverse support files') + + monkeypatch.setattr(Path, 'rglob', _unexpected_rglob) + discovered = SkillLoader().discover_skills(str(tmp_path)) + + assert list(discovered) == ['alpha@latest'] + descriptor = discovered['alpha@latest'] + assert descriptor.skill_id == 'alpha' + assert descriptor.name == 'alpha' + assert descriptor.skill_path == skill.resolve() + + +def test_discovery_matches_loader_marker_walk_rules(tmp_path): + _make_skill(tmp_path, 'top') + outer = _make_skill(tmp_path / 'group', 'outer') + _make_skill(outer / 'references', 'nested-inside-skill') + _make_skill(tmp_path / '.hidden', 'hidden') + + discovered = SkillLoader().discover_skills(str(tmp_path)) + + assert {item.skill_id for item in discovered.values()} == {'top', 'outer'} + + +def test_discovery_caches_unchanged_skill_markdown(tmp_path, monkeypatch): + _make_skill(tmp_path, 'alpha') + loader = SkillLoader() + original_read_text = Path.read_text + reads = [] + + def _read_text(path, *args, **kwargs): + reads.append(path) + return original_read_text(path, *args, **kwargs) + + monkeypatch.setattr(Path, 'read_text', _read_text) + assert loader.discover_skills(str(tmp_path)) + assert loader.discover_skills(str(tmp_path)) + + assert reads == [tmp_path / 'alpha' / 'SKILL.md'] + + +def test_full_parse_reuses_exact_legacy_files_signature(tmp_path): + skill = _make_skill(tmp_path, 'alpha') + (skill / 'z.txt').write_text('z', encoding='utf-8') + (skill / 'a').mkdir() + (skill / 'a' / 'b.txt').write_text('b', encoding='utf-8') + (skill / '.hidden').mkdir() + (skill / '.hidden' / 'ignored.txt').write_text('ignored', encoding='utf-8') + (skill / '__pycache__').mkdir() + (skill / '__pycache__' / 'tracked.pyc').write_bytes(b'bytecode') + + schema = SkillSchemaParser.parse_skill_directory(skill) + + assert schema is not None + assert schema._files_signature == _legacy_signature(skill) + + +def test_signature_changes_for_visible_resource_but_not_hidden_file(tmp_path): + skill = _make_skill(tmp_path, 'alpha') + resource = skill / 'reference.txt' + resource.write_text('before', encoding='utf-8') + first = SkillSchemaParser.parse_skill_directory(skill)._files_signature + + resource.write_text('after with different size', encoding='utf-8') + second = SkillSchemaParser.parse_skill_directory(skill)._files_signature + assert second != first + + (skill / '.transient').write_text('ignored', encoding='utf-8') + third = SkillSchemaParser.parse_skill_directory(skill)._files_signature + assert third == second From 584accbe1ff95e6001535b01d1757737e042447e Mon Sep 17 00:00:00 2001 From: alcholiclg Date: Tue, 1 Sep 2026 22:45:55 +0800 Subject: [PATCH 2/4] feat: support managed and standard skill sources --- ms_agent/config/skills_manager.py | 84 +++++++++++++++++++++++++----- ms_agent/skill/catalog.py | 18 +++++++ tests/skill/test_live_tree.py | 61 ++++++++++++++++++++-- tests/skill/test_skills_manager.py | 6 ++- 4 files changed, 151 insertions(+), 18 deletions(-) diff --git a/ms_agent/config/skills_manager.py b/ms_agent/config/skills_manager.py index 5948107f2..e11452127 100644 --- a/ms_agent/config/skills_manager.py +++ b/ms_agent/config/skills_manager.py @@ -16,12 +16,12 @@ project file — never the process cwd. The file itself keeps the raw strings (writers round-trip them untouched) so hand-written relative paths stay portable. - * Each scope has one implicit **live tree** source prepended when the - directory exists: ``/skills`` and - ``/.ms_agent/skills``. Dropping a skill directory there - registers it without touching skills.json ("existence = filesystem, - state = disabled list"). Explicit sources come after the implicit tree - so they win on skill_id collisions. + * Each scope can have two implicit discovery trees: the cross-agent standard + ``.agents/skills`` directory followed by ms-agent's managed live tree + (``/skills`` or ``/.ms_agent/skills``). Dropping a + Skill directory into either registers it without touching skills.json + ("existence = filesystem, state = disabled list"). The managed tree wins + over the standard tree, and explicit sources remain highest priority. """ from __future__ import annotations @@ -38,6 +38,22 @@ #: and ``/.ms_agent/skills``). SKILLS_TREE_DIR = 'skills' + +def global_standard_skills_tree() -> Path: + """Cross-agent personal skills: ``~/.agents/skills``. + + This directory is read-only from ms-agent's point of view. It is an + implicit discovery root, never written to ``skills.json`` and never used as + an install destination. + """ + return Path.home() / '.agents' / SKILLS_TREE_DIR + + +def project_standard_skills_tree(project_path: str) -> Path: + """Cross-agent project skills: ``/.agents/skills``.""" + root = Path(os.path.expanduser(str(project_path))).resolve() + return root / '.agents' / SKILLS_TREE_DIR + #: Remote-source prefixes that must never be path-anchored. _REMOTE_PREFIXES = ('modelscope://', 'http://', 'https://', 'git://', '@') #: ``owner/repo`` hub shorthand (mirrors sources.parse_skill_source). @@ -78,13 +94,22 @@ def __init__(self, global_dir: str = '~/.ms_agent') -> None: def load_global(self) -> Dict[str, Any]: data = self._read(self._global_path()) return self._resolved( - data, base=self._global_dir, tree=self.global_skills_tree()) + data, + base=self._global_dir, + trees=(global_standard_skills_tree(), self.global_skills_tree()), + ) def load_project(self, project_path: str) -> Dict[str, Any]: data = self._read(self._project_path(project_path)) base = Path(os.path.expanduser(str(project_path))).resolve() return self._resolved( - data, base=base, tree=self.project_skills_tree(project_path)) + data, + base=base, + trees=( + project_standard_skills_tree(project_path), + self.project_skills_tree(project_path), + ), + ) def load_merged(self, project_path: Optional[str] = None) -> Dict[str, Any]: @@ -111,18 +136,25 @@ def project_skills_tree(project_path: str) -> Path: @staticmethod def _resolved(data: Dict[str, Any], base: Path, - tree: Path) -> Dict[str, Any]: - """Anchor relative sources at *base*; prepend the live *tree* when it - exists. Preserves the empty-dict shape for missing/empty files.""" + trees: tuple[Path, ...]) -> Dict[str, Any]: + """Anchor relative sources at *base* and prepend implicit trees. + + Trees are ordered from lower to higher priority. The standard + ``.agents/skills`` root therefore comes before ms-agent's managed live + tree, while explicit sources still come last and retain their existing + precedence. + """ out = dict(data) sources = [ resolve_source_entry(s, base) for s in (data.get('sources') or []) ] implicit: List[str] = [] - if tree.is_dir(): + for tree in trees: + if not tree.is_dir(): + continue tree_str = str(tree.resolve()) - if tree_str not in sources: - implicit = [tree_str] + if tree_str not in implicit and tree_str not in sources: + implicit.append(tree_str) if implicit or sources or 'sources' in data: out['sources'] = implicit + sources return out @@ -197,6 +229,30 @@ def list_sources( return list(self.load_project(project_path).get('sources', [])) return list(self.load_global().get('sources', [])) + def list_explicit_sources( + self, + scope: str = 'global', + project_path: Optional[str] = None, + ) -> List[str]: + """Configured sources only, excluding all implicit discovery trees. + + Callers that remove a legacy path reference must not accidentally treat + ``~/.agents/skills`` or an ms-agent live tree as a removable + ``skills.json`` entry. + """ + if scope == 'project': + if not project_path: + raise ValueError('project_path required for project scope') + base = Path(os.path.expanduser(str(project_path))).resolve() + data = self._read(self._project_path(project_path)) + else: + base = self._global_dir + data = self._read(self._global_path()) + return [ + resolve_source_entry(source, base) + for source in (data.get('sources') or []) + ] + # -- internal -- def _global_path(self) -> Path: diff --git a/ms_agent/skill/catalog.py b/ms_agent/skill/catalog.py index f99f6b7f7..f8fc17901 100644 --- a/ms_agent/skill/catalog.py +++ b/ms_agent/skill/catalog.py @@ -79,6 +79,10 @@ def _download_skill_zip(skill_id: str, local_dir: str) -> str: BUILTIN_SKILLS_DIR = _candidate from ms_agent.project.paths import global_home as _global_home # noqa: E402 +from ms_agent.config.skills_manager import ( # noqa: E402 + global_standard_skills_tree, + project_standard_skills_tree, +) USER_SKILLS_DIR = _global_home() / 'skills' @@ -125,6 +129,13 @@ def load_from_config(self, skills_config) -> None: # walks it and stops at SKILL.md roots). Subdirectories are # organization, not origin: legacy installed/ & custom/ keep working # as plain subpaths. + standard_user_skills = global_standard_skills_tree() + if standard_user_skills.exists(): + sources.append( + SkillSource( + type=SkillSourceType.LOCAL_DIR, + path=str(standard_user_skills))) + if USER_SKILLS_DIR.exists(): sources.append( SkillSource( @@ -156,6 +167,13 @@ def load_from_config(self, skills_config) -> None: # 4. Workspace auto-discover (highest priority) if getattr(skills_config, 'auto_discover', False): + standard_workspace_skills = project_standard_skills_tree( + str(Path.cwd())) + if standard_workspace_skills.exists(): + sources.append( + SkillSource( + type=SkillSourceType.LOCAL_DIR, + path=str(standard_workspace_skills))) workspace_skills = Path.cwd() / 'skills' if workspace_skills.exists(): sources.append( diff --git a/tests/skill/test_live_tree.py b/tests/skill/test_live_tree.py index 303d83042..63ddcf5a3 100644 --- a/tests/skill/test_live_tree.py +++ b/tests/skill/test_live_tree.py @@ -5,8 +5,12 @@ import pytest from omegaconf import OmegaConf -from ms_agent.config.skills_manager import (SkillsConfigManager, - resolve_source_entry) +from ms_agent.config.skills_manager import ( + SkillsConfigManager, + global_standard_skills_tree, + project_standard_skills_tree, + resolve_source_entry, +) from ms_agent.skill.catalog import SkillCatalog from ms_agent.skill.loader import SkillLoader from ms_agent.skill.runtime import SkillRuntime @@ -63,7 +67,11 @@ def test_owner_repo_that_exists_locally_is_a_path(self, tmp_path): class TestManagerAnchoring: @pytest.fixture - def mgr(self, tmp_path): + def mgr(self, tmp_path, monkeypatch): + monkeypatch.setattr( + 'ms_agent.config.skills_manager.global_standard_skills_tree', + lambda: tmp_path / 'missing-standard-skills', + ) return SkillsConfigManager(global_dir=str(tmp_path / 'home')) def test_project_relative_source_anchors_at_project_root( @@ -108,6 +116,22 @@ def test_implicit_global_tree_prepended(self, mgr, tmp_path): sources = mgr.list_sources() assert sources and sources[0] == str(tree.resolve()) + def test_standard_global_tree_precedes_managed_tree( + self, mgr, tmp_path, monkeypatch): + standard = tmp_path / 'user-home' / '.agents' / 'skills' + managed = tmp_path / 'home' / 'skills' + _mk_skill(standard, 'standard') + _mk_skill(managed, 'managed') + monkeypatch.setattr( + 'ms_agent.config.skills_manager.global_standard_skills_tree', + lambda: standard, + ) + + assert mgr.list_sources()[:2] == [ + str(standard.resolve()), + str(managed.resolve()), + ] + def test_implicit_project_tree(self, mgr, tmp_path): proj = tmp_path / 'proj' tree = proj / '.ms_agent' / 'skills' @@ -115,9 +139,40 @@ def test_implicit_project_tree(self, mgr, tmp_path): sources = mgr.list_sources(scope='project', project_path=str(proj)) assert str(tree.resolve()) in sources + def test_standard_project_tree_precedes_managed_tree( + self, mgr, tmp_path): + proj = tmp_path / 'proj' + standard = project_standard_skills_tree(str(proj)) + managed = proj / '.ms_agent' / 'skills' + _mk_skill(standard, 'standard-project') + _mk_skill(managed, 'managed-project') + + sources = mgr.list_sources(scope='project', project_path=str(proj)) + assert sources[:2] == [ + str(standard.resolve()), + str(managed.resolve()), + ] + def test_no_tree_no_implicit_and_empty_shape_kept(self, mgr): assert mgr.load_global() == {} + def test_explicit_sources_exclude_implicit_trees( + self, mgr, tmp_path, monkeypatch): + standard = tmp_path / 'user-home' / '.agents' / 'skills' + managed = tmp_path / 'home' / 'skills' + explicit = tmp_path / 'external' + for root, name in ( + (standard, 'standard'), (managed, 'managed'), + (explicit, 'external')): + _mk_skill(root, name) + monkeypatch.setattr( + 'ms_agent.config.skills_manager.global_standard_skills_tree', + lambda: standard, + ) + mgr.add_source(str(explicit)) + + assert mgr.list_explicit_sources() == [str(explicit)] + def test_merged_contains_both_trees(self, mgr, tmp_path): _mk_skill(tmp_path / 'home' / 'skills', 'g') proj = tmp_path / 'proj' diff --git a/tests/skill/test_skills_manager.py b/tests/skill/test_skills_manager.py index 48f5267d7..cf5d9bcb0 100644 --- a/tests/skill/test_skills_manager.py +++ b/tests/skill/test_skills_manager.py @@ -5,7 +5,11 @@ class TestSkillsConfigManager: @pytest.fixture - def mgr(self, tmp_path): + def mgr(self, tmp_path, monkeypatch): + monkeypatch.setattr( + 'ms_agent.config.skills_manager.global_standard_skills_tree', + lambda: tmp_path / 'missing-standard-skills', + ) return SkillsConfigManager(global_dir=str(tmp_path)) def test_load_global_empty(self, mgr): From c6a3d9cca3170d401b7fc384ca7d70d267b9ecbd Mon Sep 17 00:00:00 2001 From: alcholiclg Date: Wed, 2 Sep 2026 00:50:25 +0800 Subject: [PATCH 3/4] feat: prioritize ModelScope and align provider branding --- ms_agent/llm/spec.py | 27 +++++++++++++++------------ tests/llm/test_provider_layer.py | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/ms_agent/llm/spec.py b/ms_agent/llm/spec.py index a3e0deaf3..5d3311bd7 100644 --- a/ms_agent/llm/spec.py +++ b/ms_agent/llm/spec.py @@ -116,7 +116,20 @@ def _register_builtins(self) -> None: reasoning_caps = ProviderCapabilities.from_list( ['tool_call', 'streaming', 'reasoning', 'continue_gen']) + # This order is user-visible: the WebUI preserves registry order for + # both the provider rail and provider selectors. Keep ModelScope first + # as the product's preferred built-in provider. builtins = [ + ProviderSpec( + name='modelscope', + display_name='ModelScope', + transport=TRANSPORT_OPENAI_COMPAT, + api_key_env=['MODELSCOPE_API_KEY'], + default_base_url='https://api-inference.modelscope.cn/v1', + base_url_env=['MODELSCOPE_BASE_URL'], + keywords=['qwen'], + capabilities=openai_cache_caps, + ), ProviderSpec( name='openai', display_name='OpenAI', @@ -153,16 +166,6 @@ def _register_builtins(self) -> None: keywords=['gemini-', 'gemma-'], capabilities=openai_caps, ), - ProviderSpec( - name='modelscope', - display_name='ModelScope', - transport=TRANSPORT_OPENAI_COMPAT, - api_key_env=['MODELSCOPE_API_KEY'], - default_base_url='https://api-inference.modelscope.cn/v1', - base_url_env=['MODELSCOPE_BASE_URL'], - keywords=['qwen'], - capabilities=openai_cache_caps, - ), ProviderSpec( name='zhipu', display_name='Zhipu AI (GLM)', @@ -178,7 +181,7 @@ def _register_builtins(self) -> None: ), ProviderSpec( name='kimi', - display_name='Moonshot Kimi', + display_name='Kimi (Moonshot AI)', transport=TRANSPORT_OPENAI_COMPAT, api_key_env=['KIMI_API_KEY', 'MOONSHOT_API_KEY'], default_base_url='https://api.moonshot.cn/v1', @@ -204,7 +207,7 @@ def _register_builtins(self) -> None: ), ProviderSpec( name='dashscope', - display_name='Alibaba DashScope', + display_name='Alibaba Cloud Model Studio (DashScope)', transport=TRANSPORT_OPENAI_COMPAT, api_key_env=['DASHSCOPE_API_KEY'], default_base_url= diff --git a/tests/llm/test_provider_layer.py b/tests/llm/test_provider_layer.py index fd26003d3..7e0220063 100644 --- a/tests/llm/test_provider_layer.py +++ b/tests/llm/test_provider_layer.py @@ -37,6 +37,22 @@ def test_builtins_registered(self): names = {p.name for p in get_registry().list_providers()} self.assertTrue(EXPECTED_PROVIDERS.issubset(names)) + @unittest.skipUnless(test_level() >= 0, 'skip test in current test level') + def test_modelscope_is_first_builtin(self): + providers = get_registry().list_providers() + self.assertTrue(providers) + self.assertEqual('modelscope', providers[0].name) + + @unittest.skipUnless(test_level() >= 0, 'skip test in current test level') + def test_builtin_display_names_match_product_copy(self): + display_names = { + provider.name: provider.display_name + for provider in get_registry().list_providers() + } + self.assertEqual('Kimi (Moonshot AI)', display_names['kimi']) + self.assertEqual('Alibaba Cloud Model Studio (DashScope)', + display_names['dashscope']) + @unittest.skipUnless(test_level() >= 0, 'skip test in current test level') def test_get_is_case_insensitive(self): self.assertEqual('openai', get_registry().get('OpenAI').name) From 194924519eaa0c19022ac76142a64540a9f14b69 Mon Sep 17 00:00:00 2001 From: alcholiclg Date: Wed, 2 Sep 2026 10:17:37 +0800 Subject: [PATCH 4/4] fix: shorten DashScope provider display name --- ms_agent/llm/spec.py | 2 +- tests/llm/test_provider_layer.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ms_agent/llm/spec.py b/ms_agent/llm/spec.py index 5d3311bd7..e6f25cd28 100644 --- a/ms_agent/llm/spec.py +++ b/ms_agent/llm/spec.py @@ -207,7 +207,7 @@ def _register_builtins(self) -> None: ), ProviderSpec( name='dashscope', - display_name='Alibaba Cloud Model Studio (DashScope)', + display_name='Alibaba (DashScope)', transport=TRANSPORT_OPENAI_COMPAT, api_key_env=['DASHSCOPE_API_KEY'], default_base_url= diff --git a/tests/llm/test_provider_layer.py b/tests/llm/test_provider_layer.py index 7e0220063..cc9e47167 100644 --- a/tests/llm/test_provider_layer.py +++ b/tests/llm/test_provider_layer.py @@ -50,7 +50,7 @@ def test_builtin_display_names_match_product_copy(self): for provider in get_registry().list_providers() } self.assertEqual('Kimi (Moonshot AI)', display_names['kimi']) - self.assertEqual('Alibaba Cloud Model Studio (DashScope)', + self.assertEqual('Alibaba (DashScope)', display_names['dashscope']) @unittest.skipUnless(test_level() >= 0, 'skip test in current test level')