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/llm/spec.py b/ms_agent/llm/spec.py index a3e0deaf3..e6f25cd28 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 (DashScope)', transport=TRANSPORT_OPENAI_COMPAT, api_key_env=['DASHSCOPE_API_KEY'], default_base_url= 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/llm/test_provider_layer.py b/tests/llm/test_provider_layer.py index fd26003d3..cc9e47167 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 (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) 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):