From f4451aa0ac540663d679a1d4a849e89333d78ce0 Mon Sep 17 00:00:00 2001 From: who96 <825265100@qq.com> Date: Thu, 26 Mar 2026 08:52:20 +0800 Subject: [PATCH 1/3] mcp: keep local skill search lightweight --- openspace/mcp_server.py | 65 +++++++++++++++++++++++++++++++++++- tests/test_issue3_startup.py | 49 +++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 tests/test_issue3_startup.py diff --git a/openspace/mcp_server.py b/openspace/mcp_server.py index 7e3428f..d3b8e9e 100644 --- a/openspace/mcp_server.py +++ b/openspace/mcp_server.py @@ -119,6 +119,7 @@ mcp = FastMCP("OpenSpace", **_fastmcp_kwargs) _openspace_instance = None _openspace_lock = asyncio.Lock() _standalone_store = None +_local_skill_registry = None # Internal state: tracks bot skill directories already registered this session. _registered_skill_dirs: set = set() @@ -198,6 +199,64 @@ def _get_store(): return _standalone_store +def _get_local_skill_registry(): + """Build a lightweight SkillRegistry for local-only skill search. + + This avoids initializing the full OpenSpace engine when callers only + want to inspect local skills. It mirrors the skill directory discovery + order used by the full engine, but skips LLM / provider startup. + """ + global _local_skill_registry + if _local_skill_registry is not None: + return _local_skill_registry + + from openspace.config import get_config + from openspace.skill_engine import SkillRegistry + + skill_paths: List[Path] = [] + + host_dirs_raw = os.environ.get("OPENSPACE_HOST_SKILL_DIRS", "") + if host_dirs_raw: + for d in host_dirs_raw.split(","): + d = d.strip() + if not d: + continue + p = Path(d) + if p.exists(): + skill_paths.append(p) + else: + logger.warning("Host skill dir does not exist: %s", d) + + try: + skill_cfg = get_config().skills + except Exception as e: + logger.warning("Failed to load local skill config: %s", e) + skill_cfg = None + + if skill_cfg and skill_cfg.skill_dirs: + for d in skill_cfg.skill_dirs: + p = Path(d) + if p in skill_paths: + continue + if p.exists(): + skill_paths.append(p) + else: + logger.warning("Configured skill dir does not exist: %s", d) + + builtin_skills = Path(__file__).resolve().parent / "skills" + if builtin_skills.exists(): + skill_paths.append(builtin_skills) + + if not skill_paths: + logger.debug("No local skill directories found") + return None + + registry = SkillRegistry(skill_dirs=skill_paths) + registry.discover() + _local_skill_registry = registry + return registry + + def _get_cloud_client(): """Get a OpenSpaceClient instance (raises CloudError if not configured).""" from openspace.cloud.auth import get_openspace_auth @@ -597,7 +656,11 @@ async def search_skills( # Re-scan host skill directories so newly created skills are searchable. local_skills = None store = None - if source in ("all", "local"): + if source == "local": + registry = _get_local_skill_registry() + if registry: + local_skills = registry.list_skills() + elif source == "all": openspace = await _get_openspace() host_skill_dirs_raw = os.environ.get("OPENSPACE_HOST_SKILL_DIRS", "") diff --git a/tests/test_issue3_startup.py b/tests/test_issue3_startup.py new file mode 100644 index 0000000..386a412 --- /dev/null +++ b/tests/test_issue3_startup.py @@ -0,0 +1,49 @@ +import json + +import pytest + +from openspace import mcp_server + + +@pytest.mark.asyncio +async def test_local_search_skills_does_not_initialize_openspace(monkeypatch, tmp_path): + skill_root = tmp_path / "skills" + skill_dir = skill_root / "demo-local-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + """--- +name: Demo Local Skill +description: Local regression test skill +--- + +Find me when querying demo local skill. +""", + encoding="utf-8", + ) + + async def forbidden_get_openspace(): + pytest.fail("_get_openspace should not run for source='local'") + + monkeypatch.setenv("OPENSPACE_HOST_SKILL_DIRS", str(skill_root)) + monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.setattr(mcp_server, "_get_openspace", forbidden_get_openspace) + monkeypatch.setattr( + "openspace.cloud.embedding.generate_embedding", + lambda text, api_key=None: None, + ) + + response = await mcp_server.search_skills( + query="demo local skill", + source="local", + limit=5, + auto_import=False, + ) + + payload = json.loads(response) + assert payload["count"] >= 1 + assert any( + item["name"] == "Demo Local Skill" + for item in payload["results"] + ) From fb02862d4451921c7961b48bb5e4091296f53b5e Mon Sep 17 00:00:00 2001 From: who96 <825265100@qq.com> Date: Thu, 26 Mar 2026 09:18:17 +0800 Subject: [PATCH 2/3] Refresh local registry for local skill search --- openspace/mcp_server.py | 8 ++--- tests/test_issue3_startup.py | 69 ++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/openspace/mcp_server.py b/openspace/mcp_server.py index d3b8e9e..e81ef12 100644 --- a/openspace/mcp_server.py +++ b/openspace/mcp_server.py @@ -119,7 +119,6 @@ mcp = FastMCP("OpenSpace", **_fastmcp_kwargs) _openspace_instance = None _openspace_lock = asyncio.Lock() _standalone_store = None -_local_skill_registry = None # Internal state: tracks bot skill directories already registered this session. _registered_skill_dirs: set = set() @@ -205,11 +204,9 @@ def _get_local_skill_registry(): This avoids initializing the full OpenSpace engine when callers only want to inspect local skills. It mirrors the skill directory discovery order used by the full engine, but skips LLM / provider startup. + The registry is rebuilt per call so later local searches can see + newly added skills without requiring a process restart. """ - global _local_skill_registry - if _local_skill_registry is not None: - return _local_skill_registry - from openspace.config import get_config from openspace.skill_engine import SkillRegistry @@ -253,7 +250,6 @@ def _get_local_skill_registry(): registry = SkillRegistry(skill_dirs=skill_paths) registry.discover() - _local_skill_registry = registry return registry diff --git a/tests/test_issue3_startup.py b/tests/test_issue3_startup.py index 386a412..7e1e044 100644 --- a/tests/test_issue3_startup.py +++ b/tests/test_issue3_startup.py @@ -47,3 +47,72 @@ Find me when querying demo local skill. item["name"] == "Demo Local Skill" for item in payload["results"] ) + + +@pytest.mark.asyncio +async def test_local_search_skills_refreshes_registry_between_calls(monkeypatch, tmp_path): + skill_root = tmp_path / "skills" + first_skill_dir = skill_root / "first-local-skill" + first_skill_dir.mkdir(parents=True) + (first_skill_dir / "SKILL.md").write_text( + """--- +name: First Local Skill +description: First local skill for cache regression coverage +--- + +First local skill content. +""", + encoding="utf-8", + ) + + async def forbidden_get_openspace(): + pytest.fail("_get_openspace should not run for source='local'") + + monkeypatch.setenv("OPENSPACE_HOST_SKILL_DIRS", str(skill_root)) + monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.setattr(mcp_server, "_get_openspace", forbidden_get_openspace) + monkeypatch.setattr( + "openspace.cloud.embedding.generate_embedding", + lambda text, api_key=None: None, + ) + + first_response = await mcp_server.search_skills( + query="first local skill", + source="local", + limit=5, + auto_import=False, + ) + + first_payload = json.loads(first_response) + assert any( + item["name"] == "First Local Skill" + for item in first_payload["results"] + ) + + second_skill_dir = skill_root / "second-local-skill" + second_skill_dir.mkdir(parents=True) + (second_skill_dir / "SKILL.md").write_text( + """--- +name: Second Local Skill +description: Second local skill created after the first search +--- + +Second local skill content. +""", + encoding="utf-8", + ) + + second_response = await mcp_server.search_skills( + query="second local skill", + source="local", + limit=5, + auto_import=False, + ) + + second_payload = json.loads(second_response) + assert any( + item["name"] == "Second Local Skill" + for item in second_payload["results"] + ) From 1620261783ee8676bbef5a90f017aa755e95e505 Mon Sep 17 00:00:00 2001 From: Dennis-yxchen Date: Tue, 31 Mar 2026 15:25:08 +0800 Subject: [PATCH 3/3] test: add edge case coverage for local skill search --- tests/test_issue3_startup.py | 59 ++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/test_issue3_startup.py b/tests/test_issue3_startup.py index 7e1e044..9a7b894 100644 --- a/tests/test_issue3_startup.py +++ b/tests/test_issue3_startup.py @@ -116,3 +116,62 @@ Second local skill content. item["name"] == "Second Local Skill" for item in second_payload["results"] ) + + +@pytest.mark.asyncio +async def test_local_search_returns_empty_when_no_registry(monkeypatch): + """source='local' with no discoverable skills should return empty, not crash.""" + + async def forbidden_get_openspace(): + pytest.fail("_get_openspace should not run for source='local'") + + monkeypatch.setattr(mcp_server, "_get_openspace", forbidden_get_openspace) + monkeypatch.setattr(mcp_server, "_get_local_skill_registry", lambda: None) + monkeypatch.setattr( + "openspace.cloud.embedding.generate_embedding", + lambda text, api_key=None: None, + ) + + response = await mcp_server.search_skills( + query="anything", + source="local", + limit=5, + auto_import=False, + ) + + payload = json.loads(response) + assert payload["count"] == 0 + assert payload["results"] == [] + + +@pytest.mark.asyncio +async def test_source_all_still_calls_get_openspace(monkeypatch, tmp_path): + """source='all' must go through _get_openspace(), not the lightweight path.""" + called = {"openspace": False} + + class FakeRegistry: + def list_skills(self): + return [] + + class FakeOpenSpace: + _skill_registry = FakeRegistry() + + async def tracking_get_openspace(): + called["openspace"] = True + return FakeOpenSpace() + + monkeypatch.delenv("OPENSPACE_HOST_SKILL_DIRS", raising=False) + monkeypatch.setattr(mcp_server, "_get_openspace", tracking_get_openspace) + monkeypatch.setattr( + "openspace.cloud.embedding.generate_embedding", + lambda text, api_key=None: None, + ) + + await mcp_server.search_skills( + query="anything", + source="all", + limit=5, + auto_import=False, + ) + + assert called["openspace"], "source='all' should call _get_openspace()"