From c2ee867665f774e6456a396c17cc2cd09a67f585 Mon Sep 17 00:00:00 2001 From: Alessandro <155005371+3clyp50@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:54:17 +0200 Subject: [PATCH] Add profile skill visibility policies Extend _skills with sparse allow/block rules and an explicit default for future skills, using the existing layered plugin configuration and skill-entry normalization. Enforce the policy across discovery, loading, and chat activation while preserving loaded history, legacy hidden-skill behavior, and canonical name/path identity. --- helpers/skills.py | 119 +++++++++++++++++++++++++++++++++-- helpers/skills.py.dox.md | 12 ++++ plugins/_skills/AGENTS.md | 8 ++- plugins/_skills/README.md | 7 ++- plugins/_skills/plugin.yaml | 2 +- tests/test_skills_runtime.py | 114 ++++++++++++++++++++++++++++++--- 6 files changed, 244 insertions(+), 18 deletions(-) diff --git a/helpers/skills.py b/helpers/skills.py index 87ecc9afe..c5461b714 100644 --- a/helpers/skills.py +++ b/helpers/skills.py @@ -39,6 +39,8 @@ class CatalogSkill(TypedDict): path: str origin: str hidden: bool + tags: list[str] + allowed_tools: list[str] @dataclass(slots=True) @@ -739,9 +741,70 @@ def normalize_skills_config(config: dict[str, Any] | None) -> dict[str, Any]: normalized["hidden_skills"] = normalize_hidden_skills( normalized.get("hidden_skills") ) + if "visibility_policy" in normalized: + normalized["visibility_policy"] = normalize_visibility_policy( + normalized.get("visibility_policy") + ) return normalized +def normalize_visibility_policy(raw: Any) -> dict[str, Any]: + policy = dict(raw) if isinstance(raw, dict) else {} + mode = str(policy.get("mode") or "inherit").strip().lower() + default = str(policy.get("default") or "allow").strip().lower() + policy["mode"] = "custom" if mode == "custom" else "inherit" + policy["default"] = "block" if default == "block" else "allow" + for key in ("allowed", "blocked"): + policy[key] = [ + str(entry.get("name") or entry.get("path") or "") + for entry in normalize_hidden_skills(policy.get(key)) + ] + return policy + + +def get_visibility_policy(agent: Agent | None) -> dict[str, Any]: + if not agent: + return normalize_visibility_policy(None) + config = plugin_helpers.get_plugin_config( + ACTIVE_SKILLS_PLUGIN_NAME, + agent=agent, + ) or {} + return normalize_visibility_policy(config.get("visibility_policy")) + + +def is_skill_allowed( + policy: dict[str, Any], + skill_or_entry: Skill | ActiveSkillEntry | str, +) -> bool: + if policy["mode"] != "custom": + return True + + aliases = _skill_visibility_aliases(skill_or_entry) + if any( + aliases & _skill_visibility_aliases(value) + for value in policy["blocked"] + ): + return False + if any( + aliases & _skill_visibility_aliases(value) + for value in policy["allowed"] + ): + return True + return policy["default"] == "allow" + + +def ensure_skill_visible(agent: Agent, entry: ActiveSkillEntry | str) -> None: + if is_skill_allowed(get_visibility_policy(agent), entry): + return + name = ( + str(entry.get("name") or entry.get("path") or "").strip() + if isinstance(entry, dict) + else str(entry or "").strip() + ) + profile = str(getattr(getattr(agent, "config", None), "profile", "") or "default") + raise ValueError(f'Skill "{name}" is blocked for agent profile "{profile}".') + + def normalize_active_skills( raw: Any, *, @@ -795,6 +858,7 @@ def list_skill_catalog( catalog: list[CatalogSkill] = [] seen_paths: set[str] = set() hidden_entries = get_hidden_skills(agent) if agent else [] + visibility_policy = get_visibility_policy(agent) for root in _get_catalog_roots(project_name=project_name, agent=agent): root_path = Path(root) @@ -808,6 +872,7 @@ def list_skill_catalog( continue seen_paths.add(runtime_path) + allowed = is_skill_allowed(visibility_policy, skill) catalog.append( { "name": skill.name or skill.path.name, @@ -817,7 +882,11 @@ def list_skill_catalog( runtime_path, project_name=project_name, ), - "hidden": _skill_matches_entries(skill, hidden_entries), + "hidden": _skill_matches_entries( + skill, hidden_entries + ) or not allowed, + "tags": list(skill.tags), + "allowed_tools": list(skill.allowed_tools), } ) @@ -932,12 +1001,16 @@ def _build_active_skills( current_hidden_entries, current_visible_entries, ) - return _merge_active_skill_entries( + merged = _merge_active_skill_entries( scope_entries, current_chat_entries, effective_hidden_entries, limit=effective_limit, ) + visibility_policy = get_visibility_policy(agent) + return [ + entry for entry in merged if is_skill_allowed(visibility_policy, entry) + ] def get_active_skills(agent: Agent | None) -> list[ActiveSkillEntry]: @@ -1040,6 +1113,7 @@ def activate_chat_skill(agent: Agent, entry: Any) -> list[ActiveSkillEntry]: normalized = _normalize_active_skill_entry(entry) if not normalized: raise ValueError("A skill name or path is required.") + ensure_skill_visible(agent, normalized) context = getattr(agent, "context", None) if not context: @@ -1195,6 +1269,7 @@ def show_chat_skill(agent: Agent, entry: Any) -> list[ActiveSkillEntry]: normalized = _normalize_active_skill_entry(entry) if not normalized: raise ValueError("A skill name or path is required.") + ensure_skill_visible(agent, normalized) context = getattr(agent, "context", None) if not context: @@ -1540,7 +1615,9 @@ def _skill_matches_entries( def _skill_is_hidden_for_agent(agent: Agent | None, skill: Skill) -> bool: if not agent: return False - return _skill_matches_entries(skill, get_hidden_skills(agent)) + return _skill_matches_entries( + skill, get_hidden_skills(agent) + ) or not is_skill_allowed(get_visibility_policy(agent), skill) def _filter_hidden_skills( @@ -1551,8 +1628,38 @@ def _filter_hidden_skills( return skills hidden_entries = get_hidden_skills(agent) - if not hidden_entries: - return skills + visibility_policy = get_visibility_policy(agent) return [ - skill for skill in skills if not _skill_matches_entries(skill, hidden_entries) + skill + for skill in skills + if not _skill_matches_entries(skill, hidden_entries) + and is_skill_allowed(visibility_policy, skill) ] + + +def _skill_visibility_aliases( + skill_or_entry: Skill | ActiveSkillEntry | str, +) -> set[str]: + if isinstance(skill_or_entry, Skill): + values = ( + skill_or_entry.name, + skill_or_entry.path.name, + files.normalize_a0_path(str(skill_or_entry.path)), + ) + elif isinstance(skill_or_entry, dict): + values = ( + str(skill_or_entry.get("name") or ""), + str(skill_or_entry.get("path") or ""), + ) + else: + values = (str(skill_or_entry or ""),) + + aliases: set[str] = set() + for value in values: + fixed = value.strip().replace("\\", "/").rstrip("/") + if not fixed: + continue + aliases.add(fixed.casefold()) + if "/" in fixed: + aliases.add(fixed.rsplit("/", 1)[-1].casefold()) + return aliases diff --git a/helpers/skills.py.dox.md b/helpers/skills.py.dox.md index 2d2a522e8..6149b78d5 100644 --- a/helpers/skills.py.dox.md +++ b/helpers/skills.py.dox.md @@ -41,6 +41,10 @@ - `_normalize_max_active_skills(value: Any) -> int` - `get_max_active_skills(agent: Agent | None=..., project_name: str | None=...) -> int` - `normalize_skills_config(config: dict[str, Any] | None) -> dict[str, Any]` +- `normalize_visibility_policy(raw: Any) -> dict[str, Any]` +- `get_visibility_policy(agent: Agent | None) -> dict[str, Any]` +- `is_skill_allowed(policy, skill_or_entry) -> bool` +- `ensure_skill_visible(agent, entry) -> None` - `normalize_active_skills(raw: Any, limit: int | None=...) -> list[ActiveSkillEntry]` - `normalize_hidden_skills(raw: Any) -> list[ActiveSkillEntry]` - `normalize_skill_entries(raw: Any, limit: int | None=...) -> list[ActiveSkillEntry]` @@ -60,6 +64,14 @@ - Update this file whenever public functions, classes, persistence behavior, path/security assumptions, side effects, or cross-module contracts change. - Loaded skill names are chat-wide context data under `CONTEXT_DATA_NAME_LOADED_SKILLS`; legacy agent-local `loaded_skills` lists are migrated into context data and cleared when read. - Loaded skill bodies live in chat history; hiding a skill changes catalog visibility but does not remove the loaded-skill ledger. +- `_skills.visibility_policy` is profile-aware and uses explicit future-skill + allow/block defaults. It filters discovery, search, new loading, chat + activation, and active-skill resolution without removing instructions already + stored in chat history. +- Visibility policy IDs match both canonical skill paths and their directory + names, and bulk discovery resolves the effective policy only once. +- Legacy `hidden_skills` remains default-allow with blocked exceptions; a chat + visibility override cannot bypass profile visibility policy. - `build_active_skills_prompt()` returns empty because selected skills are loaded through history, not prompt protocol. - `search_skills()` normalizes query words, scores normal terms against skill names, and scores only long terms against tags/triggers; descriptions match only full query phrases so generic prose does not produce irrelevant suggestions. - `find_skill(validate=False)` lets validation tooling resolve a skill with incomplete metadata while preserving runtime validation by default. diff --git a/plugins/_skills/AGENTS.md b/plugins/_skills/AGENTS.md index eaddb8014..e92a6052a 100644 --- a/plugins/_skills/AGENTS.md +++ b/plugins/_skills/AGENTS.md @@ -2,7 +2,8 @@ ## Purpose -- Own current-chat skill loading and hidden skill configuration. +- Own current-chat skill loading, hidden skill configuration, and profile-level + visibility policy. ## Ownership @@ -17,6 +18,11 @@ - Loaded skills are append-only from the user UI because their instructions live in chat history. - Store configured skills in normalized portable paths. - Hidden skills affect catalog/search/load visibility but must not remove loaded skill history. +- A profile visibility policy has an explicit future-skill default. It limits + discovery and new loading without pinning skills or removing history-loaded + instructions. +- Chat visibility overrides may reverse legacy `hidden_skills`, but cannot + re-enable a skill blocked by profile policy. ## Work Guidance diff --git a/plugins/_skills/README.md b/plugins/_skills/README.md index 66ecbab2a..93ed182fb 100644 --- a/plugins/_skills/README.md +++ b/plugins/_skills/README.md @@ -1,6 +1,7 @@ # Skills -Skills is a built-in Agent Zero plugin that manages skill loading and visibility for the current chat. +Skills is a built-in Agent Zero plugin that manages current-chat skill loading +and layered skill visibility, including profile-level policy from Agent Editor. ## What It Does @@ -8,7 +9,7 @@ Skills is a built-in Agent Zero plugin that manages skill loading and visibility - hides noisy skills from the model-facing available catalog, skill search, and load access - shows loaded skills without offering removal, because loaded skill bodies are part of chat history - lets users hide or show skills live per conversation -- supports global and project scoped configurations without agent-profile variants +- supports global/project settings plus profile-level Agent Editor visibility policy - links directly to the built-in Skills list - links directly to the active project's Skills section when a project is active @@ -26,3 +27,5 @@ The shared skill discovery and loaded-skill ledger live in `helpers/skills.py`, - hidden skills are stored as control data, not injected into the prompt - hidden skill paths are stored in normalized `/a0/...` form so configs stay portable across development and Docker-style layouts - if a configured hidden skill is not visible in the current agent scope, it is skipped quietly instead of breaking catalog builds +- profile visibility uses sparse Allowed/Blocked exceptions with an explicit + default for future skills; allowing a skill does not load or pin it diff --git a/plugins/_skills/plugin.yaml b/plugins/_skills/plugin.yaml index f5361e385..074a572b7 100644 --- a/plugins/_skills/plugin.yaml +++ b/plugins/_skills/plugin.yaml @@ -6,4 +6,4 @@ always_enabled: true settings_sections: - agent per_project_config: true -per_agent_config: false +per_agent_config: true diff --git a/tests/test_skills_runtime.py b/tests/test_skills_runtime.py index 9ae3984ee..88f6f87f9 100644 --- a/tests/test_skills_runtime.py +++ b/tests/test_skills_runtime.py @@ -124,6 +124,18 @@ def _scope_config(entries=None, *, hidden_entries=None, max_active_skills=None): return config +def _write_skill_catalog(root: Path, *names: str) -> Path: + skills_root = root / "skills" + for name in names: + skill_dir = skills_root / name + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {name} description\n---\nBody\n", + encoding="utf-8", + ) + return skills_root + + def test_active_skills_cap_is_twenty(): assert runtime.MAX_ACTIVE_SKILLS == 20 assert runtime.get_max_active_skills() == 20 @@ -671,14 +683,7 @@ def test_activating_new_skill_uses_scope_configured_limit(monkeypatch): def test_hidden_skills_filter_agent_visible_skill_catalog(monkeypatch, tmp_path: Path): - skills_root = tmp_path / "skills" - for name in ("alpha-skill", "beta-skill"): - skill_dir = skills_root / name - skill_dir.mkdir(parents=True) - (skill_dir / "SKILL.md").write_text( - f"---\nname: {name}\ndescription: {name} description\n---\nBody\n", - encoding="utf-8", - ) + skills_root = _write_skill_catalog(tmp_path, "alpha-skill", "beta-skill") monkeypatch.setattr( runtime.subagents, @@ -731,3 +736,96 @@ def test_chat_visible_override_restores_scope_hidden_skill(monkeypatch): runtime.hide_chat_skill(agent, {"name": "beta-skill"}) assert runtime.get_hidden_skills(agent) == [{"name": "beta-skill"}] assert runtime.get_chat_visible_skills(agent.context) == [] + + +def test_visibility_policy_is_absent_until_explicitly_configured(): + normalized = runtime.normalize_skills_config({"hidden_skills": []}) + + assert "visibility_policy" not in normalized + assert runtime.normalize_visibility_policy( + { + "mode": "custom", + "default": "block", + "allowed": ["alpha", "alpha", {"name": "missing"}], + "blocked": [], + } + ) == { + "mode": "custom", + "default": "block", + "allowed": ["alpha", "missing"], + "blocked": [], + } + + +def test_allow_only_visibility_blocks_new_skills_without_pinning( + monkeypatch, tmp_path: Path +): + skills_root = _write_skill_catalog(tmp_path, "alpha-skill", "new-skill") + + monkeypatch.setattr( + runtime.subagents, + "get_paths", + lambda agent, *parts: [str(skills_root)], + ) + monkeypatch.setattr(runtime.files, "exists", lambda path: Path(str(path)) == skills_root) + monkeypatch.setattr( + runtime.plugin_helpers, + "get_plugin_config", + lambda *args, **kwargs: { + "visibility_policy": { + "mode": "custom", + "default": "block", + "allowed": ["alpha-skill"], + "blocked": [], + } + }, + ) + agent = DummyAgent() + + assert [skill.name for skill in runtime.list_skills(agent)] == ["alpha-skill"] + assert runtime.find_skill("new-skill", agent=agent) is None + assert runtime.load_skill_for_agent("new-skill", agent=agent) == ( + "Error: skill 'new-skill' not found" + ) + assert runtime.get_active_skills(agent) == [] + + catalog = {item["name"]: item for item in runtime.list_skill_catalog(agent=agent)} + assert catalog["alpha-skill"]["hidden"] is False + assert catalog["new-skill"]["hidden"] is True + + +def test_profile_blocked_skill_cannot_be_reenabled_by_chat_override(monkeypatch): + monkeypatch.setattr( + runtime.plugin_helpers, + "get_plugin_config", + lambda *args, **kwargs: { + "visibility_policy": { + "mode": "custom", + "default": "allow", + "allowed": [], + "blocked": ["beta-skill"], + } + }, + ) + agent = DummyAgent() + + with pytest.raises(ValueError, match='Skill "beta-skill" is blocked'): + runtime.activate_chat_skill(agent, {"name": "beta-skill"}) + with pytest.raises(ValueError, match='Skill "beta-skill" is blocked'): + runtime.show_chat_skill(agent, {"name": "beta-skill"}) + with pytest.raises(ValueError, match="is blocked"): + runtime.activate_chat_skill( + agent, {"path": "/a0/skills/beta-skill"} + ) + with pytest.raises(ValueError, match="is blocked"): + runtime.show_chat_skill(agent, {"path": "/a0/skills/beta-skill"}) + + agent.context.set_data( + runtime.CONTEXT_DATA_NAME_CHAT_VISIBLE_SKILLS, + [{"name": "beta-skill"}], + ) + agent.context.set_data( + runtime.CONTEXT_DATA_NAME_CHAT_ACTIVE_SKILLS, + [{"path": "/a0/skills/beta-skill"}], + ) + assert runtime.get_active_skills(agent) == []