From 4dbb834c95678587ae9adad11b8461b536b5b8df Mon Sep 17 00:00:00 2001 From: oliver Date: Mon, 11 May 2026 16:07:34 +0800 Subject: [PATCH] Fix role binding filter when mapping is empty should_apply_workspace_role_filter no longer requires at least one bound skill. With binding enabled and an empty map, the catalog shows only public workspace skills per role (matches prewarm and runtime). Update Admin copy and add regression test. Co-authored-by: Cursor --- interfaces/admin/static/app.js | 2 +- runtime/skill_role_binding.py | 12 +++++------- tests/test_skill_role_binding_catalog.py | 24 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/interfaces/admin/static/app.js b/interfaces/admin/static/app.js index bec1e790..ac6b7c41 100644 --- a/interfaces/admin/static/app.js +++ b/interfaces/admin/static/app.js @@ -7990,7 +7990,7 @@ async function renderSkills() { class: "muted", style: "margin:8px 0;line-height:1.5;", text: - "When enabled, installed workspace skills only appear in the model skills catalog for the selected role. You can optionally inherit manager-bound skills to other roles.", + "When enabled, the model skills catalog is filtered per role: each role sees only skills bound to that role (and optionally manager-bound skills), plus skills under skills/_workspace/public/. If nothing is bound yet, non-public skills are hidden until you assign them.", }), skillBindingStatus, skillBindingPersistHint, diff --git a/runtime/skill_role_binding.py b/runtime/skill_role_binding.py index 865404a4..1ef3785b 100644 --- a/runtime/skill_role_binding.py +++ b/runtime/skill_role_binding.py @@ -128,17 +128,15 @@ def _all_installed_skill_names(store: Any) -> set[str]: def should_apply_workspace_role_filter(*, store: Any, skill_binding_role: str | None) -> bool: + """When role binding is enabled, always filter the workspace skill catalog by role. + + Empty binding maps still apply: each role then only sees ``public`` workspace skills + (see :func:`allowed_workspace_skill_names_for_role`), not the full install tree. + """ if not str(skill_binding_role or "").strip(): return False if not skill_role_binding_enabled(store=store): return False - raw = load_skill_role_binding_dict(store) - normalized = normalize_skill_role_binding( - mapping_raw=raw, - valid_skill_names=_all_installed_skill_names(store), - ) - if not mapping_has_any_skill_names(normalized): - return False return True diff --git a/tests/test_skill_role_binding_catalog.py b/tests/test_skill_role_binding_catalog.py index 8d120c4b..ea0ae9f1 100644 --- a/tests/test_skill_role_binding_catalog.py +++ b/tests/test_skill_role_binding_catalog.py @@ -95,6 +95,30 @@ def test_collect_includes_own_private_lane_without_role_mapping(tmp_path: Path, assert "lane-bound-skill" in names +def test_collect_when_binding_enabled_empty_mapping_shows_only_public(tmp_path: Path, monkeypatch) -> None: + db = tmp_path / "ops.sqlite" + store = SqliteStore(str(db)) + skills_root = tmp_path / "skills_empty_bind" + skills_root.mkdir(parents=True, exist_ok=True) + _write_skill(skills_root, "skill-root-only") + _write_skill(skills_root / "_workspace" / "public", "skill-public") + + monkeypatch.setenv("AIA_SKILLS_ROOT", str(skills_root)) + store.set_setting(SKILL_ROLE_BINDING_ENABLED_SETTING, "1") + store.set_setting(SKILL_ROLE_BINDING_KEY, "{}") + + reg = default_registry(store=store) + entries = collect_skill_catalog_entries( + store=store, + registry=reg, + base_url="", + skill_binding_role="generalist", + ) + names = {e[0] for e in entries} + assert "skill-public" in names + assert "skill-root-only" not in names + + def test_collect_unfiltered_when_binding_disabled(tmp_path: Path, monkeypatch) -> None: db = tmp_path / "ops.sqlite" store = SqliteStore(str(db))