diff --git a/runtime/workspaces/experts.py b/runtime/workspaces/experts.py index ee8974c9..58a5a808 100644 --- a/runtime/workspaces/experts.py +++ b/runtime/workspaces/experts.py @@ -15,7 +15,9 @@ _RESERVED_IDS: frozenset[str] = frozenset({"main"}) _META_FILE = "META.json" _ROLE_OPTIONS: frozenset[str] = frozenset({"system", "expert"}) _CACHE_LOCK = threading.Lock() -_LIST_CACHE_SIGNATURE: tuple[Any, ...] | None = None +_WORKSPACE_REVISION = 1 +_LIST_CACHE_REVISION: int | None = None +_LIST_CACHE_ROOT: str = "" _LIST_CACHE_ROWS: list[dict[str, Any]] = [] _CATALOG_CACHE: dict[tuple[Any, ...], str] = {} _SPECIALIST_IDS_CACHE: dict[tuple[Any, ...], tuple[str, ...]] = {} @@ -109,13 +111,16 @@ def _workspace_signature() -> tuple[Any, ...]: def expert_workspace_signature_token() -> tuple[Any, ...]: """Stable token for cache invalidation when workspace files change.""" - return _workspace_signature() + with _CACHE_LOCK: + return ("revision", int(_WORKSPACE_REVISION), str(workspaces_root())) def _clear_experts_cache() -> None: - global _LIST_CACHE_SIGNATURE, _LIST_CACHE_ROWS + global _LIST_CACHE_REVISION, _LIST_CACHE_ROOT, _LIST_CACHE_ROWS, _WORKSPACE_REVISION with _CACHE_LOCK: - _LIST_CACHE_SIGNATURE = None + _WORKSPACE_REVISION += 1 + _LIST_CACHE_REVISION = None + _LIST_CACHE_ROOT = "" _LIST_CACHE_ROWS = [] _CATALOG_CACHE.clear() _SPECIALIST_IDS_CACHE.clear() @@ -135,16 +140,19 @@ def _normalize_supported_files(files: dict[str, Any] | None) -> dict[str, str]: def list_experts() -> list[dict[str, Any]]: - global _LIST_CACHE_SIGNATURE, _LIST_CACHE_ROWS - sig = _workspace_signature() + global _LIST_CACHE_REVISION, _LIST_CACHE_ROOT, _LIST_CACHE_ROWS with _CACHE_LOCK: - if _LIST_CACHE_SIGNATURE == sig: + rev = int(_WORKSPACE_REVISION) + root_key = str(workspaces_root()) + with _CACHE_LOCK: + if _LIST_CACHE_REVISION == rev and _LIST_CACHE_ROOT == root_key: return copy.deepcopy(_LIST_CACHE_ROWS) root = workspaces_root() out: list[dict[str, Any]] = [] if not root.exists() or not root.is_dir(): with _CACHE_LOCK: - _LIST_CACHE_SIGNATURE = sig + _LIST_CACHE_REVISION = rev + _LIST_CACHE_ROOT = root_key _LIST_CACHE_ROWS = [] _CATALOG_CACHE.clear() return out @@ -183,7 +191,8 @@ def list_experts() -> list[dict[str, Any]]: } ) with _CACHE_LOCK: - _LIST_CACHE_SIGNATURE = sig + _LIST_CACHE_REVISION = rev + _LIST_CACHE_ROOT = root_key _LIST_CACHE_ROWS = copy.deepcopy(out) _CATALOG_CACHE.clear() return out @@ -197,8 +206,7 @@ def _one_line_summary(text: str, *, limit: int = 120) -> str: def build_expert_catalog_block(*, include_main: bool = False, per_field_limit: int = 120, max_total_chars: int = 4000) -> str: - sig = _workspace_signature() - cache_key = (sig, bool(include_main), int(per_field_limit), int(max_total_chars)) + cache_key = (expert_workspace_signature_token(), bool(include_main), int(per_field_limit), int(max_total_chars)) with _CACHE_LOCK: cached = _CATALOG_CACHE.get(cache_key) if isinstance(cached, str): @@ -238,8 +246,7 @@ def discover_specialist_ids_from_workspaces( *, base_order: tuple[str, ...] = ("generalist", "ops", "image", "memory"), ) -> tuple[str, ...]: - sig = _workspace_signature() - cache_key = (sig, tuple(str(x).strip().lower() for x in base_order if str(x).strip())) + cache_key = (expert_workspace_signature_token(), tuple(str(x).strip().lower() for x in base_order if str(x).strip())) with _CACHE_LOCK: cached = _SPECIALIST_IDS_CACHE.get(cache_key) if isinstance(cached, tuple): @@ -274,6 +281,28 @@ def warm_expert_workspace_cache() -> None: _ = discover_specialist_ids_from_workspaces() +def specialist_registry_snapshot( + *, + base_order: tuple[str, ...] = ("generalist", "ops", "image", "memory"), +) -> tuple[dict[str, Any], ...]: + """Single source of truth for runtime specialist discovery and metadata.""" + ordered = discover_specialist_ids_from_workspaces(base_order=base_order) + experts_by_id: dict[str, dict[str, Any]] = { + str(row.get("id") or "").strip().lower(): row for row in list_experts() if isinstance(row, dict) + } + out: list[dict[str, Any]] = [] + for sid in ordered: + row = experts_by_id.get(str(sid).strip().lower(), {}) + meta = { + "id": sid, + "role": str((row.get("role") if isinstance(row, dict) else "") or "expert").strip().lower() or "expert", + "has_required_soul": bool((row.get("has_required_soul") if isinstance(row, dict) else False)), + "builtin": bool((row.get("builtin") if isinstance(row, dict) else False)), + } + out.append(meta) + return tuple(out) + + def _workspace_dir(expert_id: str) -> Path: eid = normalize_expert_id(expert_id) if not eid: @@ -402,6 +431,7 @@ __all__ = [ "is_builtin_expert", "list_experts", "normalize_expert_id", + "specialist_registry_snapshot", "update_expert_meta", "update_expert_files", "warm_expert_workspace_cache", diff --git a/tests/test_oclaw_system_prompt.py b/tests/test_oclaw_system_prompt.py index e3eedcba..b915287d 100644 --- a/tests/test_oclaw_system_prompt.py +++ b/tests/test_oclaw_system_prompt.py @@ -127,3 +127,63 @@ def test_executor_static_prompt_cache_invalidates_on_settings_change(monkeypatch skill_binding_role="generalist", ) assert calls["n"] == 2 + + +def test_executor_static_prompt_cache_invalidates_on_workspace_revision_change(monkeypatch: pytest.MonkeyPatch) -> None: + from oclaw.runtime import system_prompt as sp + + class DummyStore: + def get_setting(self, key: str) -> str: + _ = key + return "" + + class DummyReg: + pass + + calls = {"n": 0} + token = {"v": 1} + + def _skills_block(**kwargs) -> str: + _ = kwargs + calls["n"] += 1 + return "skills-block" + + monkeypatch.setattr(sp, "expert_workspace_signature_token", lambda: ("revision", token["v"])) + monkeypatch.setattr(sp, "build_project_context_block", lambda **kwargs: "") + monkeypatch.setattr(sp, "build_skills_catalog_block", _skills_block) + monkeypatch.setattr( + sp, + "render_runtime_prompt", + lambda prompt_id, variables, strict: f"{prompt_id}\n{variables.get('skills_catalog') or ''}", + ) + + store = DummyStore() + reg = DummyReg() + _ = sp.get_executor_prompt_static( + store=store, + tools=reg, # type: ignore[arg-type] + base_url="", + base_system="base", + workspace_dir=None, + skill_binding_role="generalist", + ) + _ = sp.get_executor_prompt_static( + store=store, + tools=reg, # type: ignore[arg-type] + base_url="", + base_system="base", + workspace_dir=None, + skill_binding_role="generalist", + ) + assert calls["n"] == 1 + + token["v"] = 2 + _ = sp.get_executor_prompt_static( + store=store, + tools=reg, # type: ignore[arg-type] + base_url="", + base_system="base", + workspace_dir=None, + skill_binding_role="generalist", + ) + assert calls["n"] == 2 diff --git a/tests/test_prompt_prebuild.py b/tests/test_prompt_prebuild.py index dd8bd218..5241f680 100644 --- a/tests/test_prompt_prebuild.py +++ b/tests/test_prompt_prebuild.py @@ -101,3 +101,45 @@ def test_runtime_prewarm_prompts_snapshot_returns_roles(monkeypatch) -> None: assert "manager_system_prompt" not in prompts["manager"] assert "executor_system_prompt" not in prompts["manager"] assert "manager_user_scaffold" not in prompts["manager"] + + +def test_manager_prompt_prebuild_cache_invalidates_on_workspace_revision_change(monkeypatch) -> None: + token = {"v": 1} + calls = {"ctx": 0} + + monkeypatch.setattr(pp, "discover_specialist_ids", lambda: ("generalist", "ops")) + monkeypatch.setattr( + pp, + "list_experts", + lambda: [{"id": "generalist", "files": {"ROLE_SYSTEM.md": "General specialist"}}], + ) + monkeypatch.setattr(pp, "expert_workspace_signature_token", lambda: ("revision", token["v"])) + + def _ctx(_role: str, template_vars: dict[str, Any] | None = None) -> str: + calls["ctx"] += 1 + return f"CTX\n{str((template_vars or {}).get('MANAGER_DYNAMIC_EXPERTS_HINT') or '')}" + + monkeypatch.setattr(pp, "build_role_system_context", _ctx) + + _ = pp.get_manager_prompt_prebuild( + store=_DummyStore(), + registry=object(), + base_url="", + memory_enabled=True, + ) + _ = pp.get_manager_prompt_prebuild( + store=_DummyStore(), + registry=object(), + base_url="", + memory_enabled=True, + ) + assert calls["ctx"] == 1 + + token["v"] = 2 + _ = pp.get_manager_prompt_prebuild( + store=_DummyStore(), + registry=object(), + base_url="", + memory_enabled=True, + ) + assert calls["ctx"] == 2 diff --git a/tests/test_workspaces_experts.py b/tests/test_workspaces_experts.py index 2e3ab80d..344dfe57 100644 --- a/tests/test_workspaces_experts.py +++ b/tests/test_workspaces_experts.py @@ -14,7 +14,7 @@ def _set_project_root(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: def test_list_experts_reads_runtime_workspaces(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: _set_project_root(monkeypatch, tmp_path) - ws = tmp_path / "oclaw" / "runtime" / "workspaces" / "qa" + ws = tmp_path / "runtime" / "workspaces" / "qa" ws.mkdir(parents=True, exist_ok=True) (ws / "SOUL.md").write_text("qa soul", encoding="utf-8") rows = experts_mod.list_experts() @@ -24,7 +24,7 @@ def test_list_experts_reads_runtime_workspaces(monkeypatch: pytest.MonkeyPatch, def test_create_expert_requires_soul(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: _set_project_root(monkeypatch, tmp_path) - with pytest.raises(ValueError, match="soul_required"): + with pytest.raises(ValueError, match="soul_or_role_system_required"): experts_mod.create_expert(expert_id="qa", files={}) @@ -53,7 +53,7 @@ def test_create_update_delete_expert_flow(monkeypatch: pytest.MonkeyPatch, tmp_p def test_delete_builtin_expert_is_protected(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: _set_project_root(monkeypatch, tmp_path) - ws = tmp_path / "oclaw" / "runtime" / "workspaces" / "main" + ws = tmp_path / "runtime" / "workspaces" / "main" ws.mkdir(parents=True, exist_ok=True) (ws / "SOUL.md").write_text("main soul", encoding="utf-8") with pytest.raises(ValueError, match="builtin_expert_protected"): @@ -62,7 +62,7 @@ def test_delete_builtin_expert_is_protected(monkeypatch: pytest.MonkeyPatch, tmp def test_discover_specialists_reads_workspaces(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: _set_project_root(monkeypatch, tmp_path) - ws = tmp_path / "oclaw" / "runtime" / "workspaces" + ws = tmp_path / "runtime" / "workspaces" (ws / "main").mkdir(parents=True, exist_ok=True) (ws / "generalist").mkdir(parents=True, exist_ok=True) (ws / "generalist" / "ROLE_SYSTEM.md").write_text("g", encoding="utf-8") @@ -77,7 +77,7 @@ def test_discover_specialists_reads_workspaces(monkeypatch: pytest.MonkeyPatch, def test_build_expert_catalog_block_contains_dynamic_experts(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: _set_project_root(monkeypatch, tmp_path) - ws = tmp_path / "oclaw" / "runtime" / "workspaces" + ws = tmp_path / "runtime" / "workspaces" (ws / "main").mkdir(parents=True, exist_ok=True) (ws / "qa").mkdir(parents=True, exist_ok=True) (ws / "qa" / "SOUL.md").write_text("QA expert soul", encoding="utf-8") @@ -90,7 +90,7 @@ def test_build_expert_catalog_block_contains_dynamic_experts(monkeypatch: pytest def test_discover_specialists_cache_invalidates_after_create(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: _set_project_root(monkeypatch, tmp_path) - ws = tmp_path / "oclaw" / "runtime" / "workspaces" + ws = tmp_path / "runtime" / "workspaces" (ws / "main").mkdir(parents=True, exist_ok=True) (ws / "generalist").mkdir(parents=True, exist_ok=True) (ws / "generalist" / "SOUL.md").write_text("g", encoding="utf-8") @@ -99,3 +99,21 @@ def test_discover_specialists_cache_invalidates_after_create(monkeypatch: pytest experts_mod.create_expert(expert_id="qa", files={"SOUL.md": "qa soul"}) after = set(discover_specialist_ids()) assert "qa" in after + + +def test_workspace_revision_token_changes_after_mutation(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + _set_project_root(monkeypatch, tmp_path) + before = experts_mod.expert_workspace_signature_token() + experts_mod.create_expert(expert_id="qa", files={"SOUL.md": "qa"}) + after = experts_mod.expert_workspace_signature_token() + assert before != after + + +def test_workspace_signature_token_no_longer_scans_filesystem(monkeypatch: pytest.MonkeyPatch) -> None: + def _boom(): + raise AssertionError("should_not_scan_workspace_signature") + + monkeypatch.setattr(experts_mod, "_workspace_signature", _boom) + token = experts_mod.expert_workspace_signature_token() + assert isinstance(token, tuple) + assert token and str(token[0]) == "revision"