fix(plugins): honor plugin enable state

docs/configuration.md

@@ -2323,9 +2323,10 @@ plugins/
 ```
 
 Plugin skills use the same progressive loading and `$skill-name` invocation as workspace
-skills. A workspace skill wins when it has the same name as a plugin skill; plugin skills win
-over built-in skills. Invalid manifests, invalid Agent Skills, nested skill directories, and
-paths that resolve outside the plugin root are ignored.
+skills after the plugin is explicitly enabled. Disabling a plugin removes both its skills and
+MCP servers from the agent. A workspace skill wins when it has the same name as an enabled
+plugin skill; plugin skills win over built-in skills. Invalid manifests, invalid Agent Skills,
+nested skill directories, and paths that resolve outside the plugin root are ignored.
 
 Portable MCP servers declared in `mcp.json` appear in **Apps**, but are never started merely
 because a package exists. Enabling a plugin there is the explicit trust decision that activates
@@ -2334,15 +2335,18 @@ package paths before launch, and hot-reloads MCP connections. Explicit `tools.mc
 configuration wins over a plugin server if their host names collide. The v1 host currently
 supports plugin `stdio` servers; unsupported remote transports are skipped independently.
 
+Treat enabled plugins as local code running with the nanobot user's privileges. Manifest
+permissions are descriptive; nanobot does not currently enforce them with an OS sandbox.
+
 Plugins may optionally declare a shell-free `extensions.dev.nanobot.installCommand` array. The
 local WebUI runs it once per plugin version before first enable; remote WebUI clients cannot run
 plugin setup unless remote package installation was explicitly allowed. Agent Plugins v1 does
 not define a registry, so package distribution remains separate from discovery and execution.
 
 CLI Apps installed from the WebUI use the same package layout. nanobot installs the CLI through
-its catalog adapter, then writes a skills-only Agent Plugin under `<workspace>/plugins/`; updates
-refresh that package and uninstall removes it. The external executable remains managed by the
-CLI Apps installer rather than by the Agent Plugins manifest.
+its catalog adapter, then writes and enables a skills-only Agent Plugin under
+`<workspace>/plugins/`; updates refresh that package and uninstall removes it. The external
+executable remains managed by the CLI Apps installer rather than by the Agent Plugins manifest.
 
 ## Tool Hint Max Length
 

nanobot/agent/agent_plugins.py

@@ -12,6 +12,7 @@ from pathlib import Path
 from typing import Any, cast
 
 import yaml
+from filelock import FileLock
 from loguru import logger
 
 from nanobot.config.loader import get_config_path
@@ -41,6 +42,7 @@ _MCP_SERVER_FIELDS = {
     "stdio": {"type", "command", "args", "env", "cwd"},
 }
 _SETUP_ENV = {"HOME", "LANG", "LC_ALL", "LOGNAME", "PATH", "SHELL", "TMPDIR", "USER"}
+_SETUP_TIMEOUT_SECONDS = 600
 
 
 @dataclass(frozen=True)
@@ -111,6 +113,15 @@ def discover_agent_plugin_skills(workspace: Path) -> list[AgentPluginSkill]:
     return skills
 
 
+def enabled_agent_plugin_skills(workspace: Path) -> list[AgentPluginSkill]:
+    """Return skills from plugins the user has explicitly enabled."""
+    skills: list[AgentPluginSkill] = []
+    for plugin in discover_agent_plugins(workspace):
+        if _enabled(workspace, plugin.name):
+            skills.extend(_discover_plugin_skills(plugin.name, plugin.root))
+    return skills
+
+
 def _load_manifest(plugin_root: Path) -> AgentPlugin | None:
     manifest = _contained_file(plugin_root / "plugin.json", plugin_root)
     if manifest is None:
@@ -214,18 +225,21 @@ def agent_plugins_payload(workspace: Path) -> dict[str, Any]:
 
 
 def set_agent_plugin_enabled(workspace: Path, name: str, enabled: bool) -> dict[str, Any]:
-    """Enable or disable one installed plugin's executable MCP components."""
+    """Enable or disable one installed plugin."""
     plugin = next((item for item in discover_agent_plugins(workspace) if item.name == name), None)
     if plugin is None:
         raise ValueError(f"unknown Agent Plugin '{name}'")
     data = _plugin_data_dir(workspace, plugin.name, create=True)
-    if enabled:
-        if plugin.install_command and _setup_version(workspace, plugin.name) != (plugin.version or "unknown"):
-            _run_install(plugin, data)
-            _write_state(data / "setup-version", plugin.version or "unknown")
-        _write_state(data / "enabled", "1")
-    else:
-        (data / "enabled").unlink(missing_ok=True)
+    with FileLock(str(data / ".state.lock"), timeout=_SETUP_TIMEOUT_SECONDS + 10):
+        if enabled:
+            if plugin.install_command and _setup_version(workspace, plugin.name) != (
+                plugin.version or "unknown"
+            ):
+                _run_install(plugin, data)
+                _write_state(data / "setup-version", plugin.version or "unknown")
+            _write_state(data / "enabled", "1")
+        else:
+            (data / "enabled").unlink(missing_ok=True)
     payload = agent_plugins_payload(workspace)
     payload["last_action"] = {
         "ok": True,
@@ -478,7 +492,7 @@ def _run_install(plugin: AgentPlugin, data: Path) -> None:
             env=env,
             capture_output=True,
             text=True,
-            timeout=600,
+            timeout=_SETUP_TIMEOUT_SECONDS,
             check=False,
         )
     except subprocess.TimeoutExpired as exc:

nanobot/agent/skills.py

@@ -5,13 +5,10 @@ import os
 import re
 import shutil
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, cast
+from typing import Any, cast
 
 import yaml
 
-if TYPE_CHECKING:
-    from nanobot.agent.agent_plugins import AgentPluginSkill
-
 # Default builtin skills directory (relative to this file)
 BUILTIN_SKILLS_DIR = Path(__file__).parent.parent / "skills"
 
@@ -36,7 +33,6 @@ class SkillsLoader:
         self.workspace_skills = workspace / "skills"
         self.builtin_skills = builtin_skills_dir or BUILTIN_SKILLS_DIR
         self.disabled_skills = disabled_skills or set()
-        self.plugin_skills: list[AgentPluginSkill] = []
 
     def _skill_entries_from_dir(self, base: Path, source: str, *, skip_names: set[str] | None = None) -> list[dict[str, str]]:
         if not base.exists():
@@ -64,12 +60,12 @@ class SkillsLoader:
         Returns:
             List of skill info dicts with 'name', 'path', 'source'.
         """
-        from nanobot.agent.agent_plugins import discover_agent_plugin_skills
+        from nanobot.agent.agent_plugins import enabled_agent_plugin_skills
 
-        self.plugin_skills = discover_agent_plugin_skills(self.workspace)
+        plugin_skills = enabled_agent_plugin_skills(self.workspace)
         skills = self._skill_entries_from_dir(self.workspace_skills, "workspace")
         seen_names = {entry["name"] for entry in skills}
-        for plugin_skill in self.plugin_skills:
+        for plugin_skill in plugin_skills:
             if plugin_skill.name in seen_names:
                 continue
             skills.append(
@@ -106,11 +102,9 @@ class SkillsLoader:
         workspace_path = self.workspace_skills / name / "SKILL.md"
         if workspace_path.exists():
             return workspace_path.read_text(encoding="utf-8")
-        if not self.plugin_skills:
-            from nanobot.agent.agent_plugins import discover_agent_plugin_skills
+        from nanobot.agent.agent_plugins import enabled_agent_plugin_skills
 
-            self.plugin_skills = discover_agent_plugin_skills(self.workspace)
-        for plugin_skill in self.plugin_skills:
+        for plugin_skill in enabled_agent_plugin_skills(self.workspace):
             if plugin_skill.name == name and plugin_skill.path.is_file():
                 return plugin_skill.path.read_text(encoding="utf-8")
         if self.builtin_skills:

nanobot/apps/cli/service.py

@@ -1149,11 +1149,14 @@ Use the `run_cli_app` tool with `name="{name}"` for command execution. Do not in
             shutil.rmtree(legacy_dir)
 
     def _record_installed(self, app: dict[str, Any]) -> dict[str, Any]:
+        from nanobot.agent.agent_plugins import set_agent_plugin_enabled
+
         installed = self._load_installed()
         entry = self._installed_entry(app)
         installed[str(app["name"])] = entry
         self._save_installed(installed)
         self.install_skill(app)
+        set_agent_plugin_enabled(self.workspace, _safe_skill_name(str(app["name"])), True)
         return entry
 
     def install(self, name: str) -> dict[str, Any]:

tests/agent/test_agent_plugins.py

@@ -1,7 +1,10 @@
 import json
 import shutil
 import subprocess
+import time
+from concurrent.futures import ThreadPoolExecutor
 from pathlib import Path
+from threading import Barrier
 from typing import Any, cast
 
 import pytest
@@ -18,6 +21,15 @@ from nanobot.agent.agent_plugins import (
 from nanobot.agent.skills import SkillsLoader
 
 
+@pytest.fixture(autouse=True)
+def _isolate_plugin_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setattr(
+        agent_plugins,
+        "get_config_path",
+        lambda: tmp_path / "config" / "config.json",
+    )
+
+
 def _write_skill(root: Path, name: str, *, description: str = "Plugin skill.") -> Path:
     skill = root / "skills" / name
     skill.mkdir(parents=True)
@@ -48,6 +60,7 @@ def _write_plugin(
 def test_skills_loader_discovers_agent_plugin_skill(tmp_path: Path) -> None:
     plugin = _write_plugin(tmp_path, "acme-tools")
     _write_skill(plugin, "release-notes", description="Draft release notes from changes.")
+    set_agent_plugin_enabled(tmp_path, "acme-tools", True)
 
     loader = SkillsLoader(tmp_path, builtin_skills_dir=tmp_path / "builtin")
 
@@ -71,6 +84,7 @@ def test_skills_loader_sees_plugin_installed_after_startup(tmp_path: Path) -> No
 
     plugin = _write_plugin(tmp_path, "acme-tools")
     _write_skill(plugin, "release-notes")
+    set_agent_plugin_enabled(tmp_path, "acme-tools", True)
 
     assert [entry["name"] for entry in loader.list_skills()] == ["release-notes"]
 
@@ -152,6 +166,7 @@ def test_invalid_agent_skill_is_skipped(
 def test_workspace_skill_overrides_plugin_skill(tmp_path: Path) -> None:
     plugin = _write_plugin(tmp_path, "demo")
     _write_skill(plugin, "shared", description="Plugin version.")
+    set_agent_plugin_enabled(tmp_path, "demo", True)
     workspace_skill = tmp_path / "skills" / "shared"
     workspace_skill.mkdir(parents=True)
     (workspace_skill / "SKILL.md").write_text(
@@ -165,6 +180,36 @@ def test_workspace_skill_overrides_plugin_skill(tmp_path: Path) -> None:
     assert "Workspace version" in (loader.load_skill("shared") or "")
 
 
+def test_disabled_plugin_skill_cannot_shadow_or_inject_builtin_skill(tmp_path: Path) -> None:
+    plugin = _write_plugin(tmp_path, "demo")
+    skill = _write_skill(plugin, "shared", description="Plugin version.")
+    (skill / "SKILL.md").write_text(
+        "---\nname: shared\ndescription: Plugin version.\nalways: true\n---\n\nPlugin body.\n",
+        encoding="utf-8",
+    )
+    builtin = tmp_path / "builtin"
+    builtin_skill = builtin / "shared"
+    builtin_skill.mkdir(parents=True)
+    (builtin_skill / "SKILL.md").write_text(
+        "---\nname: shared\ndescription: Built-in version.\n---\n\nBuilt-in body.\n",
+        encoding="utf-8",
+    )
+    loader = SkillsLoader(tmp_path, builtin_skills_dir=builtin)
+
+    assert [entry["source"] for entry in loader.list_skills()] == ["builtin"]
+    assert "Built-in version" in (loader.load_skill("shared") or "")
+    assert loader.get_always_skills() == []
+
+    set_agent_plugin_enabled(tmp_path, "demo", True)
+    assert [entry["source"] for entry in loader.list_skills()] == ["plugin"]
+    assert "Plugin body" in (loader.load_skill("shared") or "")
+    assert loader.get_always_skills() == ["shared"]
+
+    set_agent_plugin_enabled(tmp_path, "demo", False)
+    assert [entry["source"] for entry in loader.list_skills()] == ["builtin"]
+    assert "Built-in version" in (loader.load_skill("shared") or "")
+
+
 def test_plugin_skill_symlink_cannot_escape_plugin_root(tmp_path: Path) -> None:
     plugin = _write_plugin(tmp_path, "demo")
     outside = tmp_path / "outside"
@@ -182,12 +227,7 @@ def test_plugin_skill_symlink_cannot_escape_plugin_root(tmp_path: Path) -> None:
     assert discover_agent_plugin_skills(tmp_path) == []
 
 
-def test_plugin_mcp_requires_explicit_enable(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
-    monkeypatch.setattr(
-        agent_plugins,
-        "get_config_path",
-        lambda: tmp_path / "config" / "config.json",
-    )
+def test_plugin_mcp_requires_explicit_enable(tmp_path: Path) -> None:
     plugin = _write_plugin(tmp_path, "desktop")
     executable = plugin / "bin" / "server"
     executable.parent.mkdir()
@@ -228,11 +268,6 @@ def test_plugin_setup_command_runs_once_per_version(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
-    monkeypatch.setattr(
-        agent_plugins,
-        "get_config_path",
-        lambda: tmp_path / "config" / "config.json",
-    )
     monkeypatch.setenv("NANOBOT_TEST_SECRET", "do-not-inherit")
     plugin = _write_plugin(
         tmp_path,
@@ -266,15 +301,46 @@ def test_plugin_setup_command_runs_once_per_version(
     assert agent_plugins_payload(tmp_path)["plugins"][0]["setup_required"] is False
 
 
-def test_invalid_plugin_mcp_entries_do_not_block_valid_servers(
+def test_concurrent_plugin_enable_runs_setup_once(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
-    monkeypatch.setattr(
-        agent_plugins,
-        "get_config_path",
-        lambda: tmp_path / "config" / "config.json",
+    plugin = _write_plugin(
+        tmp_path,
+        "desktop",
+        manifest={
+            "$schema": AGENT_PLUGIN_SCHEMA,
+            "name": "desktop",
+            "version": "1.2.3",
+            "extensions": {"dev.nanobot": {"installCommand": ["./bin/install"]}},
+        },
     )
+    executable = plugin / "bin" / "install"
+    executable.parent.mkdir()
+    executable.write_text("setup", encoding="utf-8")
+    calls: list[tuple[str, ...]] = []
+
+    def run(command: tuple[str, ...], **_: Any) -> subprocess.CompletedProcess[str]:
+        calls.append(command)
+        time.sleep(0.1)
+        return subprocess.CompletedProcess(command, 0, "ok", "")
+
+    monkeypatch.setattr(agent_plugins.subprocess, "run", run)
+    ready = Barrier(2)
+
+    def enable() -> None:
+        ready.wait()
+        set_agent_plugin_enabled(tmp_path, "desktop", True)
+
+    with ThreadPoolExecutor(max_workers=2) as executor:
+        futures = [executor.submit(enable) for _ in range(2)]
+        for future in futures:
+            future.result()
+
+    assert calls == [(str(executable),)]
+
+
+def test_invalid_plugin_mcp_entries_do_not_block_valid_servers(tmp_path: Path) -> None:
     plugin = _write_plugin(tmp_path, "network")
     executable = plugin / "bin" / "server"
     executable.parent.mkdir()

tests/cli_apps/test_service.py

@@ -9,10 +9,21 @@ from types import SimpleNamespace
 
 import pytest
 
+from nanobot.agent import agent_plugins
 from nanobot.agent.agent_plugins import discover_agent_plugin_skills
+from nanobot.agent.skills import SkillsLoader
 from nanobot.apps.cli.service import CliAppError, CliAppManager, CliAppsRuntimeConfig
 
 
+@pytest.fixture(autouse=True)
+def _isolate_plugin_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setattr(
+        agent_plugins,
+        "get_config_path",
+        lambda: tmp_path / "config" / "config.json",
+    )
+
+
 def _write_cache(path: Path, registry: dict) -> None:
     path.parent.mkdir(parents=True, exist_ok=True)
     path.write_text(
@@ -418,6 +429,11 @@ def test_install_dispatches_safe_pip_and_installs_skill(
     assert [item.name for item in discover_agent_plugin_skills(manager.workspace)] == [
         "cli-app-gimp"
     ]
+    assert [
+        item["name"]
+        for item in SkillsLoader(manager.workspace).list_skills()
+        if item["source"] == "plugin"
+    ] == ["cli-app-gimp"]
     assert not legacy.exists()