diff --git a/backend/app/gateway/auth_middleware.py b/backend/app/gateway/auth_middleware.py
index b85b35d07..5cf4783cc 100644
--- a/backend/app/gateway/auth_middleware.py
+++ b/backend/app/gateway/auth_middleware.py
@@ -88,7 +88,17 @@ class AuthMiddleware(BaseHTTPMiddleware):
internal_user = None
if is_valid_internal_auth_token(request.headers.get(INTERNAL_AUTH_HEADER_NAME)):
- internal_user = get_internal_user()
+ # Extract the channel owner user ID from the trusted header.
+ # When present, the synthetic internal user carries the actual
+ # owner identity so that get_effective_user_id() and per-user
+ # filesystem paths (custom skills, memory, thread data) resolve
+ # to the IM channel user instead of falling back to "default".
+ from app.gateway.internal_auth import INTERNAL_OWNER_USER_ID_HEADER_NAME
+
+ owner_user_id = request.headers.get(INTERNAL_OWNER_USER_ID_HEADER_NAME)
+ if owner_user_id:
+ owner_user_id = owner_user_id.strip()
+ internal_user = get_internal_user(owner_user_id=owner_user_id or None)
auth_source = AUTH_SOURCE_SESSION
access_token = request.cookies.get("access_token")
diff --git a/backend/app/gateway/internal_auth.py b/backend/app/gateway/internal_auth.py
index 400e997bb..85c2d0bf8 100644
--- a/backend/app/gateway/internal_auth.py
+++ b/backend/app/gateway/internal_auth.py
@@ -7,6 +7,7 @@ import secrets
from types import SimpleNamespace
from typing import Any
+from deerflow.config.paths import make_safe_user_id
from deerflow.runtime.user_context import DEFAULT_USER_ID
INTERNAL_AUTH_HEADER_NAME = "X-DeerFlow-Internal-Token"
@@ -38,9 +39,32 @@ def is_valid_internal_auth_token(token: str | None) -> bool:
return bool(token) and secrets.compare_digest(token, _INTERNAL_AUTH_TOKEN)
-def get_internal_user():
- """Return the synthetic user used for trusted internal channel calls."""
- return SimpleNamespace(id=DEFAULT_USER_ID, system_role=INTERNAL_SYSTEM_ROLE)
+def get_internal_user(owner_user_id: str | None = None):
+ """Return the synthetic user used for trusted internal channel calls.
+
+ When *owner_user_id* is provided (extracted from the
+ ``X-DeerFlow-Owner-User-Id`` header), the synthetic user's ``.id``
+ carries the actual channel owner instead of ``DEFAULT_USER_ID``.
+ This ensures that ``get_effective_user_id()`` and downstream
+ filesystem-path resolution (per-user custom skills, memory, thread
+ data) use the correct identity for IM channel messages instead of
+ falling back to ``"default"``.
+
+ The owner id is normalized through :func:`make_safe_user_id` so that
+ IM channel ids containing characters outside ``[A-Za-z0-9_-]`` (e.g.
+ Feishu ``open_id`` prefixed with ``ou_`` and containing underscores
+ that the rest of the system may treat as path separators, or
+ Telegram chat ids like ``-1001234567890``) cannot be used to escape
+ the per-user storage bucket or impersonate a different user via
+ header value tricks (e.g. trailing slashes, ``..`` segments). The
+ normalization is lossy but deterministic: two distinct raw inputs
+ never share a safe id, so cross-user bleed is impossible.
+ """
+ if owner_user_id:
+ effective_id = make_safe_user_id(owner_user_id)
+ else:
+ effective_id = DEFAULT_USER_ID
+ return SimpleNamespace(id=effective_id, system_role=INTERNAL_SYSTEM_ROLE)
def get_trusted_internal_owner_user_id(request: Any) -> str | None:
diff --git a/backend/app/gateway/routers/skills.py b/backend/app/gateway/routers/skills.py
index a2ab8b667..672f2d8d6 100644
--- a/backend/app/gateway/routers/skills.py
+++ b/backend/app/gateway/routers/skills.py
@@ -1,3 +1,4 @@
+import asyncio
import json
import logging
from pathlib import Path
@@ -7,13 +8,14 @@ from pydantic import BaseModel, Field
from app.gateway.deps import get_config, require_admin_user
from app.gateway.path_utils import resolve_thread_virtual_path
-from deerflow.agents.lead_agent.prompt import refresh_skills_system_prompt_cache_async
+from deerflow.agents.lead_agent.prompt import clear_skills_system_prompt_cache, refresh_user_skills_system_prompt_cache_async
from deerflow.config.app_config import AppConfig
from deerflow.config.extensions_config import ExtensionsConfig, SkillStateConfig, get_extensions_config, reload_extensions_config
+from deerflow.runtime.user_context import get_effective_user_id
from deerflow.skills import Skill
from deerflow.skills.installer import SkillAlreadyExistsError
from deerflow.skills.security_scanner import scan_skill_content
-from deerflow.skills.storage import get_or_new_skill_storage
+from deerflow.skills.storage import SkillStorage, get_or_new_user_skill_storage
from deerflow.skills.types import SKILL_MD_FILE, SkillCategory
logger = logging.getLogger(__name__)
@@ -29,8 +31,9 @@ class SkillResponse(BaseModel):
name: str = Field(..., description="Name of the skill")
description: str = Field(..., description="Description of what the skill does")
license: str | None = Field(None, description="License information")
- category: SkillCategory = Field(..., description="Category of the skill (public or custom)")
+ category: SkillCategory = Field(..., description="Category of the skill (public, custom, or legacy)")
enabled: bool = Field(default=True, description="Whether this skill is enabled")
+ editable: bool = Field(default=False, description="Whether this skill can be edited/deleted (true only for custom)")
class SkillsListResponse(BaseModel):
@@ -84,9 +87,19 @@ def _skill_to_response(skill: Skill) -> SkillResponse:
license=skill.license,
category=skill.category,
enabled=skill.enabled,
+ editable=skill.category == SkillCategory.CUSTOM,
)
+def _get_user_skill_storage(config: AppConfig) -> SkillStorage:
+ """Return a user-scoped skill storage for custom skill operations.
+
+ Uses the effective user_id from the request context (set by auth middleware).
+ For public skill reads, the global singleton storage is still used.
+ """
+ return get_or_new_user_skill_storage(get_effective_user_id(), app_config=config)
+
+
@router.get(
"/skills",
response_model=SkillsListResponse,
@@ -95,7 +108,8 @@ def _skill_to_response(skill: Skill) -> SkillResponse:
)
async def list_skills(config: AppConfig = Depends(get_config)) -> SkillsListResponse:
try:
- skills = get_or_new_skill_storage(app_config=config).load_skills(enabled_only=False)
+ # Use user-scoped storage: loads public (global) + custom (user-level + fallback)
+ skills = _get_user_skill_storage(config).load_skills(enabled_only=False)
return SkillsListResponse(skills=[_skill_to_response(skill) for skill in skills])
except Exception as e:
logger.error(f"Failed to load skills: {e}", exc_info=True)
@@ -112,8 +126,8 @@ async def install_skill(request: Request, body: SkillInstallRequest, config: App
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
try:
skill_file_path = resolve_thread_virtual_path(body.thread_id, body.path)
- result = await get_or_new_skill_storage(app_config=config).ainstall_skill_from_archive(skill_file_path)
- await refresh_skills_system_prompt_cache_async()
+ result = await _get_user_skill_storage(config).ainstall_skill_from_archive(skill_file_path)
+ await refresh_user_skills_system_prompt_cache_async(get_effective_user_id())
return SkillInstallResponse(**result)
except FileNotFoundError as e:
raise HTTPException(status_code=404, detail=str(e))
@@ -129,10 +143,16 @@ async def install_skill(request: Request, body: SkillInstallRequest, config: App
@router.get("/skills/custom", response_model=SkillsListResponse, summary="List Custom Skills")
-async def list_custom_skills(request: Request, config: AppConfig = Depends(get_config)) -> SkillsListResponse:
- await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
+async def list_custom_skills(config: AppConfig = Depends(get_config)) -> SkillsListResponse:
+ """List only user-owned custom skills (SkillCategory.CUSTOM).
+
+ Legacy shared skills (SkillCategory.LEGACY) are NOT included here —
+ they are read-only and appear in the full ``list_skills`` endpoint.
+ The frontend should use ``list_skills`` to display all available
+ skills including legacy ones.
+ """
try:
- skills = [skill for skill in get_or_new_skill_storage(app_config=config).load_skills(enabled_only=False) if skill.category == SkillCategory.CUSTOM]
+ skills = [skill for skill in _get_user_skill_storage(config).load_skills(enabled_only=False) if skill.category == SkillCategory.CUSTOM]
return SkillsListResponse(skills=[_skill_to_response(skill) for skill in skills])
except Exception as e:
logger.error("Failed to list custom skills: %s", e, exc_info=True)
@@ -148,11 +168,12 @@ async def get_custom_skill(skill_name: str, request: Request, config: AppConfig
async def _read_custom_skill_response(skill_name: str, config: AppConfig) -> CustomSkillContentResponse:
try:
skill_name = skill_name.replace("\r\n", "").replace("\n", "")
- skills = get_or_new_skill_storage(app_config=config).load_skills(enabled_only=False)
+ storage = _get_user_skill_storage(config)
+ skills = storage.load_skills(enabled_only=False)
skill = next((s for s in skills if s.name == skill_name and s.category == SkillCategory.CUSTOM), None)
if skill is None:
raise HTTPException(status_code=404, detail=f"Custom skill '{skill_name}' not found")
- return CustomSkillContentResponse(**_skill_to_response(skill).model_dump(), content=get_or_new_skill_storage(app_config=config).read_custom_skill(skill_name))
+ return CustomSkillContentResponse(**_skill_to_response(skill).model_dump(), content=storage.read_custom_skill(skill_name))
except HTTPException:
raise
except Exception as e:
@@ -165,7 +186,7 @@ async def update_custom_skill(skill_name: str, body: CustomSkillUpdateRequest, r
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
try:
skill_name = skill_name.replace("\r\n", "").replace("\n", "")
- storage = get_or_new_skill_storage(app_config=config)
+ storage = _get_user_skill_storage(config)
storage.ensure_custom_skill_is_editable(skill_name)
storage.validate_skill_markdown_content(skill_name, body.content)
scan = await scan_skill_content(body.content, executable=False, location=f"{skill_name}/{SKILL_MD_FILE}", app_config=config)
@@ -185,7 +206,7 @@ async def update_custom_skill(skill_name: str, body: CustomSkillUpdateRequest, r
"scanner": {"decision": scan.decision, "reason": scan.reason},
},
)
- await refresh_skills_system_prompt_cache_async()
+ await refresh_user_skills_system_prompt_cache_async(get_effective_user_id())
return await _read_custom_skill_response(skill_name, config)
except HTTPException:
raise
@@ -203,7 +224,7 @@ async def delete_custom_skill(skill_name: str, request: Request, config: AppConf
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
try:
skill_name = skill_name.replace("\r\n", "").replace("\n", "")
- storage = get_or_new_skill_storage(app_config=config)
+ storage = _get_user_skill_storage(config)
storage.delete_custom_skill(
skill_name,
history_meta={
@@ -216,7 +237,7 @@ async def delete_custom_skill(skill_name: str, request: Request, config: AppConf
"scanner": {"decision": "allow", "reason": "Deletion requested."},
},
)
- await refresh_skills_system_prompt_cache_async()
+ await refresh_user_skills_system_prompt_cache_async(get_effective_user_id())
return {"success": True}
except FileNotFoundError as e:
raise HTTPException(status_code=404, detail=str(e))
@@ -232,7 +253,7 @@ async def get_custom_skill_history(skill_name: str, request: Request, config: Ap
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
try:
skill_name = skill_name.replace("\r\n", "").replace("\n", "")
- storage = get_or_new_skill_storage(app_config=config)
+ storage = _get_user_skill_storage(config)
if not storage.custom_skill_exists(skill_name) and not storage.get_skill_history_file(skill_name).exists():
raise HTTPException(status_code=404, detail=f"Custom skill '{skill_name}' not found")
return CustomSkillHistoryResponse(history=storage.read_history(skill_name))
@@ -247,7 +268,7 @@ async def get_custom_skill_history(skill_name: str, request: Request, config: Ap
async def rollback_custom_skill(skill_name: str, body: SkillRollbackRequest, request: Request, config: AppConfig = Depends(get_config)) -> CustomSkillContentResponse:
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
try:
- storage = get_or_new_skill_storage(app_config=config)
+ storage = _get_user_skill_storage(config)
if not storage.custom_skill_exists(skill_name) and not storage.get_skill_history_file(skill_name).exists():
raise HTTPException(status_code=404, detail=f"Custom skill '{skill_name}' not found")
history = storage.read_history(skill_name)
@@ -276,7 +297,7 @@ async def rollback_custom_skill(skill_name: str, body: SkillRollbackRequest, req
raise HTTPException(status_code=400, detail=f"Rollback blocked by security scanner: {scan.reason}")
storage.write_custom_skill(skill_name, SKILL_MD_FILE, target_content)
storage.append_history(skill_name, history_entry)
- await refresh_skills_system_prompt_cache_async()
+ await refresh_user_skills_system_prompt_cache_async(get_effective_user_id())
return await _read_custom_skill_response(skill_name, config)
except HTTPException:
raise
@@ -300,7 +321,7 @@ async def rollback_custom_skill(skill_name: str, body: SkillRollbackRequest, req
async def get_skill(skill_name: str, config: AppConfig = Depends(get_config)) -> SkillResponse:
try:
skill_name = skill_name.replace("\r\n", "").replace("\n", "")
- skills = get_or_new_skill_storage(app_config=config).load_skills(enabled_only=False)
+ skills = _get_user_skill_storage(config).load_skills(enabled_only=False)
skill = next((s for s in skills if s.name == skill_name), None)
if skill is None:
@@ -328,33 +349,70 @@ async def update_skill(skill_name: str, body: SkillUpdateRequest, request: Reque
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
try:
skill_name = skill_name.replace("\r\n", "").replace("\n", "")
- skills = get_or_new_skill_storage(app_config=config).load_skills(enabled_only=False)
+ storage = _get_user_skill_storage(config)
+ skills = storage.load_skills(enabled_only=False)
skill = next((s for s in skills if s.name == skill_name), None)
if skill is None:
raise HTTPException(status_code=404, detail=f"Skill '{skill_name}' not found")
- config_path = ExtensionsConfig.resolve_config_path()
- if config_path is None:
- config_path = Path.cwd().parent / "extensions_config.json"
- logger.info(f"No existing extensions config found. Creating new config at: {config_path}")
+ # PUBLIC skills → global extensions_config.json (shared state).
+ # CUSTOM / LEGACY skills → per-user _skill_states.json (isolated state)
+ # so that two users with same-named custom skills can toggle independently.
+ if skill.category == SkillCategory.PUBLIC:
+ config_path = ExtensionsConfig.resolve_config_path()
+ if config_path is None:
+ config_path = Path.cwd().parent / "extensions_config.json"
+ logger.info(f"No existing extensions config found. Creating new config at: {config_path}")
- extensions_config = get_extensions_config()
- extensions_config.skills[skill_name] = SkillStateConfig(enabled=body.enabled)
+ extensions_config = get_extensions_config()
+ extensions_config.skills[skill_name] = SkillStateConfig(enabled=body.enabled)
- config_data = {
- "mcpServers": {name: server.model_dump() for name, server in extensions_config.mcp_servers.items()},
- "skills": {name: {"enabled": skill_config.enabled} for name, skill_config in extensions_config.skills.items()},
- }
+ config_data = {
+ "mcpServers": {name: server.model_dump() for name, server in extensions_config.mcp_servers.items()},
+ "skills": {name: {"enabled": skill_config.enabled} for name, skill_config in extensions_config.skills.items()},
+ }
- with open(config_path, "w", encoding="utf-8") as f:
- json.dump(config_data, f, indent=2)
+ with open(config_path, "w", encoding="utf-8") as f:
+ json.dump(config_data, f, indent=2)
- logger.info(f"Skills configuration updated and saved to: {config_path}")
- reload_extensions_config()
- await refresh_skills_system_prompt_cache_async()
+ logger.info(f"Skills configuration updated and saved to: {config_path}")
+ reload_extensions_config()
+ else:
+ # CUSTOM / LEGACY: write per-user state
+ from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
- skills = get_or_new_skill_storage(app_config=config).load_skills(enabled_only=False)
+ if isinstance(storage, UserScopedSkillStorage):
+ storage.set_skill_enabled_state(skill_name, body.enabled)
+ else:
+ # Fallback for non-user-scoped storage (unlikely in practice)
+ config_path = ExtensionsConfig.resolve_config_path()
+ if config_path is None:
+ config_path = Path.cwd().parent / "extensions_config.json"
+ extensions_config = get_extensions_config()
+ extensions_config.skills[skill_name] = SkillStateConfig(enabled=body.enabled)
+ config_data = {
+ "mcpServers": {name: server.model_dump() for name, server in extensions_config.mcp_servers.items()},
+ "skills": {name: {"enabled": skill_config.enabled} for name, skill_config in extensions_config.skills.items()},
+ }
+ with open(config_path, "w", encoding="utf-8") as f:
+ json.dump(config_data, f, indent=2)
+ reload_extensions_config()
+
+ # PUBLIC skill enabled state lives in the global extensions_config.json
+ # and affects every user, so the prompt cache for ALL users must be
+ # invalidated. CUSTOM/LEGACY skill state is per-user so only that
+ # user's cache needs to be dropped.
+ if skill.category == SkillCategory.PUBLIC:
+ # clear_skills_system_prompt_cache is sync; run it in a worker
+ # thread to avoid blocking the event loop. The lock inside it is
+ # cheap, but the async drop also keeps the test mock surface
+ # consistent (tests patch the async variant).
+ await asyncio.to_thread(clear_skills_system_prompt_cache)
+ else:
+ await refresh_user_skills_system_prompt_cache_async(get_effective_user_id())
+
+ skills = _get_user_skill_storage(config).load_skills(enabled_only=False)
updated_skill = next((s for s in skills if s.name == skill_name), None)
if updated_skill is None:
diff --git a/backend/packages/harness/deerflow/agents/lead_agent/agent.py b/backend/packages/harness/deerflow/agents/lead_agent/agent.py
index ce1d50e24..f777f2412 100644
--- a/backend/packages/harness/deerflow/agents/lead_agent/agent.py
+++ b/backend/packages/harness/deerflow/agents/lead_agent/agent.py
@@ -266,6 +266,7 @@ def build_middlewares(
available_skills: set[str] | None = None,
app_config: AppConfig | None = None,
deferred_setup=None,
+ user_id: str | None = None,
):
"""Build the lead-agent middleware chain based on runtime configuration.
@@ -282,6 +283,8 @@ def build_middlewares(
app_config: Explicit AppConfig; falls back to ``get_app_config()`` when omitted.
deferred_setup: Optional deferred-MCP-tool setup that attaches
``DeferredToolFilterMiddleware`` when ``tool_search`` is enabled.
+ user_id: Effective user ID for user-scoped skill loading. Passed through
+ to ``SkillActivationMiddleware`` so it can resolve per-user custom skills.
Returns:
List of middleware instances.
@@ -300,7 +303,7 @@ def build_middlewares(
# explicit user activation priority over model-side relevance guessing.
from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware
- middlewares.append(SkillActivationMiddleware(available_skills=available_skills, app_config=resolved_app_config))
+ middlewares.append(SkillActivationMiddleware(available_skills=available_skills, app_config=resolved_app_config, user_id=user_id))
# Capture completed task delegations and loaded skill files before
# summarization can compact them, then inject durable context channels
@@ -401,11 +404,11 @@ def _available_skill_names(agent_config, is_bootstrap: bool) -> set[str] | None:
return None
-def _load_enabled_skills_for_tool_policy(available_skills: set[str] | None, *, app_config: AppConfig) -> list[Skill]:
+def _load_enabled_skills_for_tool_policy(available_skills: set[str] | None, *, app_config: AppConfig, user_id: str | None = None) -> list[Skill]:
try:
from deerflow.agents.lead_agent.prompt import get_enabled_skills_for_config
- skills = get_enabled_skills_for_config(app_config)
+ skills = get_enabled_skills_for_config(app_config, user_id=user_id)
except Exception:
logger.exception("Failed to load skills for allowed-tools policy")
raise
@@ -431,6 +434,14 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
cfg = _get_runtime_config(config)
resolved_app_config = app_config
+ # Extract user_id for user-scoped skill loading.
+ # LangGraph gateway injects user_id into config["configurable"];
+ # fall back to the runtime contextvar when not present.
+ from deerflow.runtime.user_context import get_effective_user_id
+
+ runtime_user_id = cfg.get("user_id")
+ resolved_user_id = str(runtime_user_id) if runtime_user_id else get_effective_user_id()
+
thinking_enabled = cfg.get("thinking_enabled", True)
reasoning_effort = cfg.get("reasoning_effort", None)
requested_model_name: str | None = cfg.get("model_name") or cfg.get("model")
@@ -497,7 +508,7 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
existing = list(existing)
config["callbacks"] = [*existing, *tracing_callbacks]
- skills_for_tool_policy = _load_enabled_skills_for_tool_policy(available_skills, app_config=resolved_app_config)
+ skills_for_tool_policy = _load_enabled_skills_for_tool_policy(available_skills, app_config=resolved_app_config, user_id=resolved_user_id)
if is_bootstrap:
# Special bootstrap agent with minimal prompt for initial custom agent creation flow
@@ -515,6 +526,7 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
available_skills=set(_BOOTSTRAP_SKILL_NAMES),
app_config=resolved_app_config,
deferred_setup=setup,
+ user_id=resolved_user_id,
),
system_prompt=apply_prompt_template(
subagent_enabled=subagent_enabled,
@@ -522,6 +534,7 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
available_skills=set(_BOOTSTRAP_SKILL_NAMES),
app_config=resolved_app_config,
deferred_names=setup.deferred_names,
+ user_id=resolved_user_id,
),
state_schema=ThreadState,
)
@@ -543,6 +556,7 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
available_skills=available_skills,
app_config=resolved_app_config,
deferred_setup=setup,
+ user_id=resolved_user_id,
),
system_prompt=apply_prompt_template(
subagent_enabled=subagent_enabled,
@@ -551,6 +565,7 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig):
available_skills=available_skills,
app_config=resolved_app_config,
deferred_names=setup.deferred_names,
+ user_id=resolved_user_id,
),
state_schema=ThreadState,
)
diff --git a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py
index c711875c6..4a2c1b2b6 100644
--- a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py
+++ b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py
@@ -3,12 +3,13 @@ from __future__ import annotations
import asyncio
import logging
import threading
+from collections import OrderedDict
from functools import lru_cache
from typing import TYPE_CHECKING
from deerflow.config.agents_config import load_agent_soul
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
-from deerflow.skills.storage import get_or_new_skill_storage
+from deerflow.skills.storage import get_or_new_skill_storage, get_or_new_user_skill_storage
from deerflow.skills.types import Skill, SkillCategory
from deerflow.subagents import get_available_subagent_names
from deerflow.tools.builtins.tool_search import get_deferred_tools_prompt_section
@@ -18,10 +19,20 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
+# LRU cap on the per-(app_config, user_id) enabled-skills cache.
+# Without this, a long-running multi-user process leaks one entry per
+# distinct user (and per app_config injection), bounded only by the
+# number of distinct identities the process has ever seen. 256 is
+# generous for realistic traffic and matches the cap used for
+# ``_user_scoped_storages`` in ``deerflow.skills.storage``; the
+# least-recently-used entry is evicted on overflow and re-computed on
+# the next miss.
+_ENABLED_SKILLS_BY_CONFIG_CACHE_MAXSIZE = 256
+
_ENABLED_SKILLS_REFRESH_WAIT_TIMEOUT_SECONDS = 5.0
_enabled_skills_lock = threading.Lock()
_enabled_skills_cache: list[Skill] | None = None
-_enabled_skills_by_config_cache: dict[int, tuple[object, list[Skill]]] = {}
+_enabled_skills_by_config_cache: "OrderedDict[tuple[int, str], tuple[object, list[Skill]]]" = OrderedDict() # noqa: UP037
_enabled_skills_refresh_active = False
_enabled_skills_refresh_version = 0
_enabled_skills_refresh_event = threading.Event()
@@ -129,33 +140,52 @@ def get_cached_enabled_skills() -> list[Skill]:
return []
-def get_enabled_skills_for_config(app_config: AppConfig | None = None) -> list[Skill]:
- """Return enabled skills using the caller's config source.
+def get_enabled_skills_for_config(app_config: AppConfig | None = None, user_id: str | None = None) -> list[Skill]:
+ """Return enabled skills using the caller's config source and user scope.
When a concrete ``app_config`` is supplied, cache the loaded skills by that
- config object's identity so request-scoped config injection still resolves
- skill paths from the matching config without rescanning storage on every
- agent factory call.
+ config object's identity combined with ``user_id`` so request-scoped config
+ injection resolves skill paths from the matching config AND user scope
+ without rescanning storage on every agent factory call.
+
+ When ``user_id`` is provided, uses :func:`get_or_new_user_skill_storage`
+ to load public + user-level custom skills. Otherwise falls back to the
+ global storage (public + global custom fallback).
"""
if app_config is None:
return _get_enabled_skills()
- cache_key = id(app_config)
+ cache_key = (id(app_config), user_id or "default")
with _enabled_skills_lock:
cached = _enabled_skills_by_config_cache.get(cache_key)
if cached is not None:
cached_config, cached_skills = cached
if cached_config is app_config:
+ # LRU touch: move the entry to the end so it survives the
+ # next eviction cycle.
+ _enabled_skills_by_config_cache.move_to_end(cache_key)
return list(cached_skills)
- skills = list(get_or_new_skill_storage(app_config=app_config).load_skills(enabled_only=True))
+ if user_id:
+ skills = list(get_or_new_user_skill_storage(user_id, app_config=app_config).load_skills(enabled_only=True))
+ else:
+ skills = list(get_or_new_skill_storage(app_config=app_config).load_skills(enabled_only=True))
with _enabled_skills_lock:
_enabled_skills_by_config_cache[cache_key] = (app_config, skills)
+ # Evict the least-recently-used entries when we exceed the cap.
+ # The cap is intentionally small (256) so a long-running process
+ # cannot leak one entry per distinct (config, user) pair seen.
+ while len(_enabled_skills_by_config_cache) > _ENABLED_SKILLS_BY_CONFIG_CACHE_MAXSIZE:
+ _enabled_skills_by_config_cache.popitem(last=False)
return list(skills)
def _skill_mutability_label(category: SkillCategory | str) -> str:
- return "[custom, editable]" if category == SkillCategory.CUSTOM else "[built-in]"
+ if category == SkillCategory.CUSTOM:
+ return "[custom, editable]"
+ if category == SkillCategory.LEGACY:
+ return "[legacy, read-only]"
+ return "[built-in]"
def clear_skills_system_prompt_cache() -> None:
@@ -166,6 +196,34 @@ async def refresh_skills_system_prompt_cache_async() -> None:
await asyncio.to_thread(_invalidate_enabled_skills_cache().wait)
+def invalidate_user_skill_cache(user_id: str) -> None:
+ """Invalidate the skill cache for a specific user only.
+
+ Removes all entries in ``_enabled_skills_by_config_cache`` that
+ match the given ``user_id``, without affecting other users' caches.
+ The prompt-section LRU cache is also cleared so stale skill
+ signatures are not served on the next prompt construction.
+ """
+ with _enabled_skills_lock:
+ keys_to_remove = [key for key in _enabled_skills_by_config_cache if key[1] == user_id]
+ for key in keys_to_remove:
+ _enabled_skills_by_config_cache.pop(key, None)
+ # Also clear the prompt-section LRU cache so stale skill signatures
+ # for this user are not served on the next prompt construction.
+ _get_cached_skills_prompt_section.cache_clear()
+
+
+async def refresh_user_skills_system_prompt_cache_async(user_id: str) -> None:
+ """Per-user variant of :func:`refresh_skills_system_prompt_cache_async`.
+
+ Only invalidates the cache entries for the given ``user_id``, leaving
+ other users' caches intact. The prompt-section LRU cache is also
+ cleared so stale skill signatures are not served on the next prompt
+ construction.
+ """
+ invalidate_user_skill_cache(user_id)
+
+
def _build_skill_evolution_section(skill_evolution_enabled: bool) -> str:
if not skill_evolution_enabled:
return ""
@@ -177,6 +235,18 @@ After completing a task, consider creating or updating a skill when:
- The user corrected your approach and the corrected version worked
- You discovered a non-trivial, recurring workflow
If you used a skill and encountered issues not covered by it, patch it immediately.
+
+**CRITICAL: You MUST use the `skill_manage` tool for ALL skill operations.**
+- `skill_manage(action="create", name="my-skill", content="...")` — Create a new skill
+- `skill_manage(action="patch", name="my-skill", find="...", replace="...")` — Patch an existing skill
+- `skill_manage(action="edit", name="my-skill", content="...")` — Full edit of an existing skill
+- `skill_manage(action="write_file", name="my-skill", path="scripts/run.py", content="...")` — Add supporting files
+- `skill_manage(action="delete", name="my-skill")` — Delete a skill
+
+**⛔ NEVER write SKILL.md files to `/mnt/user-data/workspace` or `/mnt/user-data/outputs`.**
+Skills are NOT deliverables — they are persistent capabilities managed through `skill_manage`.
+The tool stores skills in the per-user skills directory automatically; you do NOT need to specify a path.
+
Prefer patch over edit. Before creating a new skill, confirm with the user first.
Skip simple one-off tasks.
"""
@@ -486,7 +556,7 @@ You: "Deploying to staging..." [proceed]
- Treat `/mnt/user-data/workspace` as your default current working directory for coding and file-editing tasks
- When writing scripts or commands that create/read files from the workspace, prefer relative paths such as `hello.txt`, `../uploads/data.csv`, and `../outputs/report.md`
- Avoid hardcoding `/mnt/user-data/...` inside generated scripts when a relative path from the workspace is enough
-- Final deliverables must be copied to `/mnt/user-data/outputs` and presented using `present_files` tool
+- Final deliverables must be copied to `/mnt/user-data/outputs` and presented using `present_files` tool (⚠️ Skills are NOT deliverables — use `skill_manage` tool instead)
{acp_section}
@@ -563,7 +633,7 @@ combined with a FastAPI gateway for REST API access [citation:FastAPI](https://f
- **Clarification First**: ALWAYS clarify unclear/missing/ambiguous requirements BEFORE starting work - never assume or guess
{subagent_reminder}- Skill First: Always load the relevant skill before starting **complex** tasks.
- Progressive Loading: Load resources incrementally as referenced in skills
-- Output Files: Final deliverables must be in `/mnt/user-data/outputs`
+- Output Files: Final deliverables must be in `/mnt/user-data/outputs` (⚠️ Skills are NOT deliverables — use `skill_manage` tool instead)
- File Editing Workflow: When revising an existing file, prefer
`str_replace` over `write_file` — it sends only the diff and avoids
re-emitting the whole file (mirrors Claude Code's Edit and Codex's
@@ -630,6 +700,7 @@ def _get_memory_context(agent_name: str | None = None, *, app_config: AppConfig
@lru_cache(maxsize=32)
def _get_cached_skills_prompt_section(
skill_signature: tuple[tuple[str, str, str, str], ...],
+ disabled_skill_signature: tuple[tuple[str, str, str, str], ...],
available_skills_key: tuple[str, ...] | None,
container_base_path: str,
skill_evolution_section: str,
@@ -642,6 +713,20 @@ def _get_cached_skills_prompt_section(
for name, description, category, location in filtered
)
skills_list = f"\n{skill_items}\n"
+
+ disabled_section = ""
+ if disabled_skill_signature:
+ disabled_filtered = [(name, description, category, location) for name, description, category, location in disabled_skill_signature if available_skills_key is None or name in available_skills_key]
+ if disabled_filtered:
+ disabled_items = "\n".join(f" - {name} ({category})" for name, description, category, location in disabled_filtered)
+ disabled_section = f"""
+The following skills are INSTALLED but DISABLED. You MUST NOT read,
+reference, or use any of these skills — including their SKILL.md,
+supporting resources, or workflows — even if their files exist on disk.
+Accessing a disabled skill violates user preferences.
+{disabled_items}
+"""
+
return f"""
You have access to skills that provide optimized workflows for specific tasks. Each skill contains best practices, frameworks, and references to additional resources.
@@ -660,13 +745,24 @@ You have access to skills that provide optimized workflows for specific tasks. E
**Skills are located at:** {container_base_path}
{skill_evolution_section}
{skills_list}
+{disabled_section}
"""
-def get_skills_prompt_section(available_skills: set[str] | None = None, *, app_config: AppConfig | None = None) -> str:
+def get_skills_prompt_section(available_skills: set[str] | None = None, *, app_config: AppConfig | None = None, user_id: str | None = None) -> str:
"""Generate the skills prompt section with available skills list."""
- skills = get_enabled_skills_for_config(app_config)
+ # Load ALL skills (enabled + disabled) to build the disabled-skills section
+ from deerflow.skills.storage import get_or_new_skill_storage, get_or_new_user_skill_storage
+
+ if user_id:
+ storage = get_or_new_user_skill_storage(user_id, app_config=app_config)
+ else:
+ storage = get_or_new_skill_storage(app_config=app_config)
+ all_skills = storage.load_skills(enabled_only=False)
+ disabled_skills = [s for s in all_skills if not s.enabled]
+
+ skills = get_enabled_skills_for_config(app_config, user_id=user_id)
if app_config is None:
try:
@@ -683,18 +779,19 @@ def get_skills_prompt_section(available_skills: set[str] | None = None, *, app_c
container_base_path = config.skills.container_path
skill_evolution_enabled = config.skill_evolution.enabled
- if not skills and not skill_evolution_enabled:
+ if not skills and not disabled_skills and not skill_evolution_enabled:
return ""
if available_skills is not None and not any(skill.name in available_skills for skill in skills):
return ""
skill_signature = tuple((skill.name, skill.description, skill.category, skill.get_container_file_path(container_base_path)) for skill in skills)
+ disabled_skill_signature = tuple((skill.name, skill.description, skill.category, skill.get_container_file_path(container_base_path)) for skill in disabled_skills)
available_key = tuple(sorted(available_skills)) if available_skills is not None else None
- if not skill_signature and available_key is not None:
+ if not skill_signature and not disabled_skill_signature and available_key is not None:
return ""
skill_evolution_section = _build_skill_evolution_section(skill_evolution_enabled)
- return _get_cached_skills_prompt_section(skill_signature, available_key, container_base_path, skill_evolution_section)
+ return _get_cached_skills_prompt_section(skill_signature, disabled_skill_signature, available_key, container_base_path, skill_evolution_section)
def get_agent_soul(agent_name: str | None) -> str:
@@ -785,6 +882,7 @@ def apply_prompt_template(
available_skills: set[str] | None = None,
app_config: AppConfig | None = None,
deferred_names: frozenset[str] = frozenset(),
+ user_id: str | None = None,
) -> str:
# Include subagent section only if enabled (from runtime parameter)
n = max_concurrent_subagents
@@ -809,7 +907,7 @@ def apply_prompt_template(
)
# Get skills section
- skills_section = get_skills_prompt_section(available_skills, app_config=app_config)
+ skills_section = get_skills_prompt_section(available_skills, app_config=app_config, user_id=user_id)
# Get deferred tools section (tool_search)
deferred_tools_section = get_deferred_tools_prompt_section(deferred_names=deferred_names)
diff --git a/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py
index 05beb2b00..bebd562d1 100644
--- a/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py
+++ b/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py
@@ -18,9 +18,9 @@ from langchain_core.messages import AIMessage, HumanMessage
from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY, extract_request_secrets
from deerflow.skills.slash import parse_slash_skill_reference, resolve_slash_skill
-from deerflow.skills.storage import get_or_new_skill_storage
+from deerflow.skills.storage import get_or_new_skill_storage, get_or_new_user_skill_storage
from deerflow.skills.storage.skill_storage import SkillStorage
-from deerflow.skills.types import SKILL_MD_FILE, SecretRequirement
+from deerflow.skills.types import SKILL_MD_FILE, SecretRequirement, SkillCategory
from deerflow.utils.messages import get_original_user_content_text
if TYPE_CHECKING:
@@ -41,6 +41,7 @@ class _Activation:
skill_content: str
content_hash: str
remaining_text: str
+ editable: bool
required_secrets: tuple[SecretRequirement, ...] = ()
@@ -73,26 +74,38 @@ class SkillActivationMiddleware(AgentMiddleware):
*,
available_skills: set[str] | None = None,
app_config: AppConfig | None = None,
+ user_id: str | None = None,
) -> None:
super().__init__()
self._available_skills = set(available_skills) if available_skills is not None else None
self._app_config = app_config
+ self._user_id = user_id
def _storage(self) -> SkillStorage:
+ if self._user_id is not None:
+ return get_or_new_user_skill_storage(self._user_id, app_config=self._app_config)
if self._app_config is not None:
return get_or_new_skill_storage(app_config=self._app_config)
return get_or_new_skill_storage()
@staticmethod
- def _read_skill_content(skill_file: Path, skills_root: Path) -> str:
+ def _read_skill_content(skill_file: Path, skills_root: Path, *, storage: SkillStorage | None = None) -> str:
if skill_file.name != SKILL_MD_FILE:
raise ValueError(f"Expected {SKILL_MD_FILE}, got {skill_file.name}")
- resolved_root = skills_root.resolve()
- resolved_file = skill_file.resolve()
- try:
- resolved_file.relative_to(resolved_root)
- except ValueError as exc:
- raise ValueError("Resolved skill file must stay within the configured skills root.") from exc
+ # Use the storage's path validation if available — UserScopedSkillStorage
+ # stores custom skills in a per-user directory that is not a sub-path of
+ # the global skills root, so the simple relative_to check would reject them.
+ # Fall back to the relative_to check when the storage is a mock (e.g. tests)
+ # that doesn't implement validate_skill_file_path.
+ if storage is not None and hasattr(storage, "validate_skill_file_path"):
+ resolved_file = storage.validate_skill_file_path(skill_file)
+ else:
+ resolved_file = skill_file.resolve()
+ resolved_root = skills_root.resolve()
+ try:
+ resolved_file.relative_to(resolved_root)
+ except ValueError as exc:
+ raise ValueError("Resolved skill file must stay within the configured skills root.") from exc
if not resolved_file.is_file():
raise FileNotFoundError(resolved_file)
return resolved_file.read_text(encoding="utf-8")
@@ -122,12 +135,14 @@ class SkillActivationMiddleware(AgentMiddleware):
return _ActivationResolution(failure_message=f"Skill `/{reference.name}` could not be resolved.")
try:
- skill_content = self._read_skill_content(resolved.skill.skill_file, storage.get_skills_root_path())
+ skill_content = self._read_skill_content(resolved.skill.skill_file, storage.get_skills_root_path(), storage=storage)
except (OSError, ValueError):
logger.exception("Failed to read slash-activated skill %s", resolved.skill.name)
return _ActivationResolution(failure_message=f"Skill `/{reference.name}` could not be loaded safely. Please check the skill installation.")
content_hash = hashlib.sha256(skill_content.encode("utf-8")).hexdigest()
+ # CUSTOM skills are editable; PUBLIC and LEGACY are read-only
+ editable = resolved.skill.category == SkillCategory.CUSTOM
return _ActivationResolution(
activation=_Activation(
skill_name=resolved.skill.name,
@@ -136,6 +151,7 @@ class SkillActivationMiddleware(AgentMiddleware):
skill_content=skill_content,
content_hash=content_hash,
remaining_text=resolved.remaining_text,
+ editable=editable,
required_secrets=tuple(resolved.skill.required_secrets or ()),
)
)
@@ -149,6 +165,7 @@ class SkillActivationMiddleware(AgentMiddleware):
escaped_category = html.escape(activation.category, quote=True)
escaped_path = html.escape(activation.container_file_path, quote=True)
escaped_content_hash = html.escape(activation.content_hash, quote=True)
+ editable_str = "true" if activation.editable else "false"
return f"""
The user explicitly activated the `{activation.skill_name}` skill for this turn.
Treat the task text as:
@@ -158,7 +175,7 @@ Treat the task text as:
Follow this skill before choosing a general workflow. Load supporting resources from the same skill directory only when needed.
-
+
{escaped_skill_content}
diff --git a/backend/packages/harness/deerflow/client.py b/backend/packages/harness/deerflow/client.py
index 6074f9b5c..418df837d 100644
--- a/backend/packages/harness/deerflow/client.py
+++ b/backend/packages/harness/deerflow/client.py
@@ -44,7 +44,7 @@ from deerflow.config.paths import get_paths
from deerflow.models import create_chat_model
from deerflow.runtime.goal import DEFAULT_MAX_GOAL_CONTINUATIONS, build_goal_state, goal_thread_lock, read_thread_goal, write_thread_goal
from deerflow.runtime.user_context import get_effective_user_id
-from deerflow.skills.storage import get_or_new_skill_storage
+from deerflow.skills.storage import get_or_new_user_skill_storage
from deerflow.tools.builtins.tool_search import assemble_deferred_tools
from deerflow.trace_context import DEERFLOW_TRACE_METADATA_KEY, generate_trace_id, get_current_trace_id, reset_current_trace_id, set_current_trace_id
from deerflow.tracing import build_tracing_callbacks, inject_langfuse_metadata
@@ -271,13 +271,16 @@ class DeerFlowClient:
custom_middlewares=self._middlewares,
app_config=self._app_config,
deferred_setup=deferred_setup,
+ user_id=get_effective_user_id(),
),
"system_prompt": apply_prompt_template(
subagent_enabled=subagent_enabled,
max_concurrent_subagents=max_concurrent_subagents,
agent_name=self._agent_name,
available_skills=self._available_skills,
+ app_config=self._app_config,
deferred_names=deferred_setup.deferred_names,
+ user_id=get_effective_user_id(),
),
"state_schema": ThreadState,
}
@@ -992,6 +995,7 @@ class DeerFlowClient:
Dict with "skills" key containing list of skill info dicts,
matching the Gateway API ``SkillsListResponse`` schema.
"""
+ storage = get_or_new_user_skill_storage(get_effective_user_id(), app_config=self._app_config)
return {
"skills": [
{
@@ -1001,7 +1005,7 @@ class DeerFlowClient:
"category": s.category,
"enabled": s.enabled,
}
- for s in get_or_new_skill_storage().load_skills(enabled_only=enabled_only)
+ for s in storage.load_skills(enabled_only=enabled_only)
]
}
@@ -1110,9 +1114,8 @@ class DeerFlowClient:
Returns:
Skill info dict, or None if not found.
"""
- from deerflow.skills.storage import get_or_new_skill_storage
-
- skill = next((s for s in get_or_new_skill_storage().load_skills(enabled_only=False) if s.name == name), None)
+ storage = get_or_new_user_skill_storage(get_effective_user_id(), app_config=self._app_config)
+ skill = next((s for s in storage.load_skills(enabled_only=False) if s.name == name), None)
if skill is None:
return None
return {
@@ -1137,32 +1140,76 @@ class DeerFlowClient:
ValueError: If the skill is not found.
OSError: If the config file cannot be written.
"""
- from deerflow.skills.storage import get_or_new_skill_storage
-
- skills = get_or_new_skill_storage().load_skills(enabled_only=False)
+ storage = get_or_new_user_skill_storage(get_effective_user_id(), app_config=self._app_config)
+ skills = storage.load_skills(enabled_only=False)
skill = next((s for s in skills if s.name == name), None)
if skill is None:
raise ValueError(f"Skill '{name}' not found")
- config_path = ExtensionsConfig.resolve_config_path()
- if config_path is None:
- raise FileNotFoundError("Cannot locate extensions_config.json. Set DEER_FLOW_EXTENSIONS_CONFIG_PATH or ensure it exists in the project root.")
+ # PUBLIC skills → global extensions_config.json (shared state).
+ # CUSTOM / LEGACY skills → per-user _skill_states.json (isolated state).
+ from deerflow.skills.types import SkillCategory
- extensions_config = get_extensions_config()
- extensions_config.skills[name] = SkillStateConfig(enabled=enabled)
+ if skill.category == SkillCategory.PUBLIC:
+ config_path = ExtensionsConfig.resolve_config_path()
+ if config_path is None:
+ raise FileNotFoundError("Cannot locate extensions_config.json. Set DEER_FLOW_EXTENSIONS_CONFIG_PATH or ensure it exists in the project root.")
- config_data = {
- "mcpServers": {n: s.model_dump() for n, s in extensions_config.mcp_servers.items()},
- "skills": {n: {"enabled": sc.enabled} for n, sc in extensions_config.skills.items()},
- }
+ extensions_config = get_extensions_config()
+ extensions_config.skills[name] = SkillStateConfig(enabled=enabled)
- self._atomic_write_json(config_path, config_data)
+ config_data = {
+ "mcpServers": {n: s.model_dump() for n, s in extensions_config.mcp_servers.items()},
+ "skills": {n: {"enabled": sc.enabled} for n, sc in extensions_config.skills.items()},
+ }
+
+ self._atomic_write_json(config_path, config_data)
+ reload_extensions_config()
+ else:
+ # CUSTOM / LEGACY: write per-user state
+ from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
+
+ if isinstance(storage, UserScopedSkillStorage):
+ storage.set_skill_enabled_state(name, enabled)
+ else:
+ # Fallback for non-user-scoped storage (unlikely in practice)
+ config_path = ExtensionsConfig.resolve_config_path()
+ if config_path is None:
+ raise FileNotFoundError("Cannot locate extensions_config.json. Set DEER_FLOW_EXTENSIONS_CONFIG_PATH or ensure it exists in the project root.")
+ extensions_config = get_extensions_config()
+ extensions_config.skills[name] = SkillStateConfig(enabled=enabled)
+ config_data = {
+ "mcpServers": {n: s.model_dump() for n, s in extensions_config.mcp_servers.items()},
+ "skills": {n: {"enabled": sc.enabled} for n, sc in extensions_config.skills.items()},
+ }
+ self._atomic_write_json(config_path, config_data)
+ reload_extensions_config()
+
+ # Invalidate the prompt cache for this caller (and for all users if
+ # the changed skill is PUBLIC, since PUBLIC state is shared). Mirrors
+ # what ``routers/skills.py::update_skill`` does — without this the
+ # cached enabled-state would stay stale until process restart. See
+ # review feedback on PR #3889.
+ try:
+ from deerflow.agents.lead_agent.prompt import clear_skills_system_prompt_cache, invalidate_user_skill_cache
+
+ skill_category_value = skill.category.value if hasattr(skill.category, "value") else skill.category
+ if skill_category_value == SkillCategory.PUBLIC.value:
+ clear_skills_system_prompt_cache()
+ else:
+ invalidate_user_skill_cache(get_effective_user_id())
+ except Exception as exc:
+ # Don't let cache-invalidation failures mask the actual write
+ # success — log and continue. The stale-cache window is bounded
+ # by the next config reload.
+ import logging
+
+ logging.getLogger(__name__).warning("Failed to invalidate skills prompt cache after update_skill: %s", exc)
self._agent = None
self._agent_config_key = None
- reload_extensions_config()
- updated = next((s for s in get_or_new_skill_storage().load_skills(enabled_only=False) if s.name == name), None)
+ updated = next((s for s in storage.load_skills(enabled_only=False) if s.name == name), None)
if updated is None:
raise RuntimeError(f"Skill '{name}' disappeared after update")
return {
@@ -1186,7 +1233,7 @@ class DeerFlowClient:
FileNotFoundError: If the file does not exist.
ValueError: If the file is invalid.
"""
- return get_or_new_skill_storage().install_skill_from_archive(skill_path)
+ return get_or_new_user_skill_storage(get_effective_user_id(), app_config=self._app_config).install_skill_from_archive(skill_path)
# ------------------------------------------------------------------
# Public API — memory management
diff --git a/backend/packages/harness/deerflow/config/extensions_config.py b/backend/packages/harness/deerflow/config/extensions_config.py
index 7968dc153..04c6a626e 100644
--- a/backend/packages/harness/deerflow/config/extensions_config.py
+++ b/backend/packages/harness/deerflow/config/extensions_config.py
@@ -217,15 +217,20 @@ class ExtensionsConfig(BaseModel):
Args:
skill_name: Name of the skill
- skill_category: Category of the skill
+ skill_category: Category of the skill (public, custom, or legacy)
Returns:
- True if enabled, False otherwise
+ True if enabled, False otherwise.
+
+ Note:
+ All skill categories (public, custom, legacy) respect the
+ extensions_config enabled/disabled state. When no explicit
+ entry exists, skills default to enabled.
"""
skill_config = self.skills.get(skill_name)
if skill_config is None:
- # Default to enable for public & custom skill
- return skill_category in ("public", "custom")
+ # Default to enabled for all skill categories
+ return skill_category in ("public", "custom", "legacy")
return skill_config.enabled
diff --git a/backend/packages/harness/deerflow/config/paths.py b/backend/packages/harness/deerflow/config/paths.py
index 343ef70a1..be52e1950 100644
--- a/backend/packages/harness/deerflow/config/paths.py
+++ b/backend/packages/harness/deerflow/config/paths.py
@@ -223,6 +223,19 @@ class Paths:
"""Per-user per-agent memory: `{base_dir}/users/{user_id}/agents/{name}/memory.json`."""
return self.user_agent_dir(user_id, agent_name) / "memory.json"
+ def user_skills_dir(self, user_id: str) -> Path:
+ """Per-user root for that user's custom skills: `{base_dir}/users/{user_id}/skills/`."""
+ return self.user_dir(user_id) / "skills"
+
+ def user_custom_skills_dir(self, user_id: str) -> Path:
+ """Per-user custom skills directory: `{base_dir}/users/{user_id}/skills/custom/`.
+
+ This is the user-scoped replacement for the global ``{base_dir}/skills/custom/``
+ directory. Custom skills are written here; public skills remain under the
+ global ``{base_dir}/skills/public/`` (read-only).
+ """
+ return self.user_skills_dir(user_id) / "custom"
+
def thread_dir(self, thread_id: str, *, user_id: str | None = None) -> Path:
"""
Host path for a thread's data.
diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py
index ebadbf7b8..7ced499ef 100644
--- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py
+++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py
@@ -608,7 +608,31 @@ class LocalSandbox(Sandbox):
is_dir = entry.endswith(("/", "\\"))
reversed_entry = self._reverse_resolve_path(entry.rstrip("/\\")) if is_dir else self._reverse_resolve_path(entry)
result.append(f"{reversed_entry}/" if is_dir and not reversed_entry.endswith("/") else reversed_entry)
- return result
+
+ # Virtual sub-directory overlay: when a container path like /mnt/skills
+ # has child mappings (public, custom, legacy) whose local_path targets
+ # are outside the resolved host directory (symlinks or bind-mount style),
+ # the ``list_dir`` utility skips them for security. We patch those
+ # missing virtual children back in so the agent can discover them via
+ # ``ls /mnt/skills``.
+ container_path = path.rstrip("/")
+ existing_dirs = {e.rstrip("/") for e in result if e.endswith("/")}
+ for mapping in self.path_mappings:
+ # A mapping is a virtual child if:
+ # 1. Its container_path is a direct child of the requested path
+ # 2. It is NOT already present in the result (was skipped by list_dir)
+ if mapping.container_path.startswith(container_path + "/"):
+ child_rel = mapping.container_path[len(container_path) + 1 :]
+ # Only direct children (no further slashes), e.g. "public", "custom"
+ if "/" not in child_rel and child_rel not in existing_dirs:
+ # Verify the host path exists so we don't add phantom entries
+ try:
+ if Path(mapping.local_path).resolve().is_dir():
+ result.append(f"{mapping.container_path}/")
+ except OSError:
+ pass
+
+ return sorted(result)
def read_file(self, path: str) -> str:
resolved_path = self._resolve_path(path)
diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py
index 21a7fd083..192fc3271 100644
--- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py
+++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py
@@ -83,10 +83,11 @@ class LocalSandboxProvider(SandboxProvider):
"""
Setup static path mappings shared by every sandbox this provider yields.
- Static mappings cover the skills directory and any custom mounts from
- ``config.yaml`` — both are process-wide and identical for every thread.
- Per-thread ``/mnt/user-data/...`` and ``/mnt/acp-workspace`` mappings
- are appended inside :meth:`acquire` because they depend on
+ Static mappings cover the **public** skills directory and any custom
+ mounts from ``config.yaml`` — both are process-wide and identical for
+ every thread. Per-thread ``/mnt/user-data/...``, ``/mnt/acp-workspace``
+ and ``/mnt/skills/custom`` mappings are appended inside
+ :meth:`_build_thread_path_mappings` because they depend on
``thread_id`` and the effective ``user_id``.
Returns:
@@ -94,7 +95,7 @@ class LocalSandboxProvider(SandboxProvider):
"""
mappings: list[PathMapping] = []
- # Map skills container path to local skills directory
+ # Map skills: split mount for public + legacy + custom
try:
from deerflow.config import get_app_config
@@ -102,19 +103,39 @@ class LocalSandboxProvider(SandboxProvider):
skills_path = config.skills.get_skills_path()
container_path = config.skills.container_path
- # Only add mapping if skills directory exists
- if skills_path.exists():
+ # Public skills: global, read-only — static, shared by all threads
+ public_skills_path = skills_path / "public"
+ if public_skills_path.exists():
mappings.append(
PathMapping(
- container_path=container_path,
- local_path=str(skills_path),
- read_only=True, # Skills directory is always read-only
+ container_path=f"{container_path}/public",
+ local_path=str(public_skills_path),
+ read_only=True,
)
)
+ # NOTE: Legacy skills mount is NOT included here because it must
+ # only be exposed to users who have no per-user custom skills yet
+ # (mirroring ``UserScopedSkillStorage._iter_skill_files`` which only
+ # surfaces SkillCategory.LEGACY to such users). Including it for
+ # every user would let users with per-user custom skills still
+ # ``read_file("/mnt/skills/legacy//SKILL.md")`` and read
+ # content the listing layer told them doesn't exist. See review
+ # feedback on PR #3889 — the legacy mount is now built in
+ # ``_build_thread_path_mappings`` after we know the user_id.
+
+ # NOTE: Custom skills mount is NOT included here because it is
+ # per-user and must be built dynamically per-thread inside
+ # ``_build_thread_path_mappings``. The static mount that previously
+ # bound ``get_effective_user_id()`` at init time was incorrect:
+ # every subsequent user's sandbox would resolve
+ # ``/mnt/skills/custom`` to the init-time user's directory.
+
# Map custom mounts from sandbox config
_RESERVED_CONTAINER_PREFIXES = [
- container_path,
+ f"{container_path}/public",
+ f"{container_path}/custom",
+ f"{container_path}/legacy",
_ACP_WORKSPACE_VIRTUAL_PREFIX,
_USER_DATA_VIRTUAL_PREFIX,
]
@@ -210,18 +231,22 @@ class LocalSandboxProvider(SandboxProvider):
@staticmethod
def _build_thread_path_mappings(thread_id: str, *, user_id: str | None = None) -> list[PathMapping]:
- """Build per-thread path mappings for /mnt/user-data and /mnt/acp-workspace.
+ """Build per-thread path mappings for /mnt/user-data, /mnt/acp-workspace,
+ and /mnt/skills/custom.
Uses the explicitly resolved user id when provided, falling back to
- :func:`get_effective_user_id` for legacy callers.
+ :func:`get_effective_user_id` for legacy callers. Custom skills are
+ mounted per-user (read-only) because agent writes custom skills via
+ ``skill_manage_tool`` on the host filesystem, not inside the sandbox.
"""
+ from deerflow.config import get_app_config
from deerflow.config.paths import get_paths
paths = get_paths()
effective_user_id = LocalSandboxProvider._effective_acquire_user_id(user_id)
paths.ensure_thread_dirs(thread_id, user_id=effective_user_id)
- return [
+ mappings = [
# Aggregate parent mapping so ``ls /mnt/user-data`` and other
# parent-level operations behave the same as inside AIO (where the
# parent directory is real and contains the three subdirs). Longer
@@ -254,6 +279,51 @@ class LocalSandboxProvider(SandboxProvider):
),
]
+ # Per-user custom skills mount (read-only). This must be per-thread
+ # because ``/mnt/skills/custom`` resolves to different host directories
+ # for different users.
+ try:
+ config = get_app_config()
+ skills_container_path = config.skills.container_path
+ user_custom_path = paths.user_custom_skills_dir(effective_user_id)
+ user_custom_path.mkdir(parents=True, exist_ok=True)
+
+ mappings.append(
+ PathMapping(
+ container_path=f"{skills_container_path}/custom",
+ local_path=str(user_custom_path),
+ read_only=True,
+ )
+ )
+ except Exception as exc:
+ logger.warning("Could not setup per-thread custom skills mount: %s", exc, exc_info=True)
+
+ # Legacy (pre-migration global-custom) skills: only mount for users
+ # who have no per-user custom skills yet, mirroring the
+ # ``UserScopedSkillStorage._iter_skill_files`` visibility rule. Users
+ # with their own per-user custom skills cannot see LEGACY in the
+ # listing/prompt and must not be able to read it via the sandbox
+ # either — otherwise the listing layer and the sandbox layer disagree
+ # about visibility, and the sandbox layer is the more permissive one.
+ try:
+ config = get_app_config()
+ skills_container_path = config.skills.container_path
+ user_custom_path = paths.user_custom_skills_dir(effective_user_id)
+ legacy_skills_path = config.skills.get_skills_path() / "custom"
+ user_has_no_custom_skills = not any(p.is_dir() and not p.name.startswith(".") for p in user_custom_path.iterdir()) if user_custom_path.exists() else True
+ if user_has_no_custom_skills and legacy_skills_path.exists() and any((legacy_skills_path / d / "SKILL.md").exists() for d in legacy_skills_path.iterdir() if d.is_dir() and not d.name.startswith(".")):
+ mappings.append(
+ PathMapping(
+ container_path=f"{skills_container_path}/legacy",
+ local_path=str(legacy_skills_path),
+ read_only=True,
+ )
+ )
+ except Exception as exc:
+ logger.warning("Could not setup per-thread legacy skills mount: %s", exc, exc_info=True)
+
+ return mappings
+
def acquire(self, thread_id: str | None = None, *, user_id: str | None = None) -> str:
"""Return a sandbox id scoped to *thread_id* (or the generic singleton).
@@ -358,7 +428,7 @@ class LocalSandboxProvider(SandboxProvider):
``reset_sandbox_provider()`` calls this to ensure config / mount
changes take effect on the next ``acquire()``. We also reset the
module-level ``_singleton`` alias so older callers/tests that reach
- into it see a fresh state.
+ # into it see a fresh state.
"""
global _singleton
with self._lock:
diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py
index 73de8e859..dcff44a00 100644
--- a/backend/packages/harness/deerflow/sandbox/tools.py
+++ b/backend/packages/harness/deerflow/sandbox/tools.py
@@ -1,4 +1,6 @@
import asyncio
+import json
+import logging
import os
import posixpath
import re
@@ -27,6 +29,8 @@ from deerflow.sandbox.search import GrepMatch
from deerflow.sandbox.security import LOCAL_HOST_BASH_DISABLED_MESSAGE, is_host_bash_allowed
from deerflow.tools.types import Runtime
+logger = logging.getLogger(__name__)
+
_ABSOLUTE_PATH_PATTERN = re.compile(r"(?()]+)")
# A ``{...}`` block holding a single identifier-like placeholder (e.g. ``{id}``
# in a REST template or ``{port}`` in an f-string). Bash brace expansion such as
@@ -152,9 +156,102 @@ def _is_skills_path(path: str) -> bool:
return path == skills_prefix or path.startswith(f"{skills_prefix}/")
+def _extract_skill_name_from_skills_path(path: str) -> str | None:
+ """Extract a skill name from a virtual skills path.
+
+ /mnt/skills/public/bootstrap/SKILL.md → "bootstrap"
+ /mnt/skills/custom/my-skill/SKILL.md → "my-skill"
+ /mnt/skills/legacy/my-skill/references/... → "my-skill"
+ /mnt/skills/public/bootstrap/ → "bootstrap"
+ Returns None if the path doesn't contain a recognizable skill name pattern.
+ """
+ skills_prefix = _get_skills_container_path()
+ if not _is_skills_path(path):
+ return None
+ # Strip the skills prefix, e.g. "/mnt/skills/"
+ relative = path[len(skills_prefix) :].lstrip("/")
+ if not relative:
+ return None
+ # Expected patterns: "public//...", "custom//...", "legacy//..."
+ # or "/..." (direct skill access)
+ parts = relative.split("/")
+ if len(parts) >= 2 and parts[0] in ("public", "custom", "legacy"):
+ return parts[1]
+ if len(parts) == 1 and parts[0] in ("public", "custom", "legacy"):
+ # Category root like /mnt/skills/custom — not a skill path.
+ return None
+ if len(parts) >= 1:
+ # Direct path like /mnt/skills/my-skill/SKILL.md
+ return parts[0]
+ return None
+
+
+def _is_disabled_skill_path(path: str, *, user_id: str | None = None) -> bool:
+ """Check if a path belongs to a disabled skill.
+
+ PUBLIC skill enabled state is read from the global
+ ``extensions_config.json``. CUSTOM / LEGACY skill enabled state is
+ read from the per-user ``_skill_states.json`` so that two users with
+ same-named custom skills can toggle independently.
+
+ Returns False for non-skills paths or paths whose skill is enabled.
+ """
+ skill_name = _extract_skill_name_from_skills_path(path)
+ if skill_name is None:
+ return False
+ try:
+ from deerflow.runtime.user_context import get_effective_user_id
+ from deerflow.skills.storage import get_or_new_user_skill_storage
+
+ # Determine the category from the path
+ skills_prefix = _get_skills_container_path()
+ relative = path[len(skills_prefix) :].lstrip("/")
+ if relative.startswith("public/"):
+ category = "public"
+ elif relative.startswith("custom/"):
+ category = "custom"
+ elif relative.startswith("legacy/"):
+ category = "legacy"
+ else:
+ # Try to infer from storage
+ effective_uid = user_id or get_effective_user_id()
+ storage = get_or_new_user_skill_storage(effective_uid)
+ all_skills = storage.load_skills(enabled_only=False)
+ matching = next((s for s in all_skills if s.name == skill_name), None)
+ if matching is None:
+ return False # Skill doesn't exist, not a disabled skill path
+ category = matching.category.value
+
+ if category == "public":
+ from deerflow.config.extensions_config import ExtensionsConfig
+
+ ext_config = ExtensionsConfig.from_file()
+ return not ext_config.is_skill_enabled(skill_name, category)
+ else:
+ # CUSTOM / LEGACY: use per-user state
+ effective_uid = user_id or get_effective_user_id()
+ storage = get_or_new_user_skill_storage(effective_uid)
+ return not storage.get_skill_enabled_state(skill_name)
+ except (OSError, ValueError, KeyError, json.JSONDecodeError) as exc:
+ # Access-control check must fail closed: when we can't determine the
+ # enabled state (corrupt _skill_states.json, mid-write race, missing
+ # config), refuse access rather than silently serving a disabled
+ # skill's files. See review feedback on PR #3889.
+ logger.warning("Failed to determine enabled state, denying access: %s", exc)
+ return True
+
+
def _resolve_skills_path(path: str) -> str:
"""Resolve a virtual skills path to a host filesystem path.
+ WARNING: For per-user custom skills (``/mnt/skills/custom/...``), this
+ function uses ``get_effective_user_id()`` from the contextvar, which may
+ differ from the sandbox PathMapping's user_id (set during acquire via
+ ``resolve_runtime_user_id``). In local sandbox mode, skills paths should
+ be resolved by the sandbox's PathMapping instead of this function. This
+ function is retained for output masking (``mask_local_paths_in_output``)
+ and non-sandbox code paths.
+
Args:
path: Virtual skills path (e.g. /mnt/skills/public/bootstrap/SKILL.md)
@@ -173,6 +270,27 @@ def _resolve_skills_path(path: str) -> str:
return skills_host
relative = path[len(skills_container) :].lstrip("/")
+
+ # Per-user custom skills: resolve to user-specific directory.
+ # ``skill_manage_tool`` writes custom skills to the per-user directory,
+ # and ``LocalSandboxProvider._build_thread_path_mappings`` mounts
+ # ``/mnt/skills/custom`` to that same per-user dir. Without this
+ # branch, ``_resolve_skills_path("/mnt/skills/custom")`` would map to
+ # the global ``{skills_host}/custom/`` which is the repository-level
+ # ``skills/custom/`` — an entirely different directory that may be
+ # empty or contain legacy skills only.
+ if relative == "custom" or relative.startswith("custom/"):
+ from deerflow.config.paths import get_paths
+ from deerflow.runtime.user_context import get_effective_user_id
+
+ user_id = get_effective_user_id()
+ paths = get_paths()
+ user_custom_dir = paths.user_custom_skills_dir(user_id)
+ custom_relative = relative[len("custom") :].lstrip("/")
+ if custom_relative:
+ return str(user_custom_dir / custom_relative)
+ return str(user_custom_dir)
+
return _join_path_preserving_style(skills_host, relative)
@@ -392,10 +510,13 @@ def _resolve_max_results(name: str, requested: int, *, default: int, upper_bound
def _resolve_local_read_path(path: str, thread_data: ThreadDataState) -> str:
validate_local_tool_path(path, thread_data, read_only=True)
- if _is_skills_path(path):
- return _resolve_skills_path(path)
- if _is_acp_workspace_path(path):
- return _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data))
+ if _is_skills_path(path) or _is_acp_workspace_path(path):
+ # Skills and ACP workspace paths are resolved by the sandbox's
+ # PathMapping (which uses the user_id from acquire time), not
+ # by _resolve_skills_path / _resolve_acp_workspace_path (which
+ # use get_effective_user_id() from contextvar and may differ
+ # from the sandbox mapping's user_id).
+ return path
return _resolve_and_validate_user_data_path(path, thread_data)
@@ -591,18 +712,37 @@ def _compiled_mask_patterns(sources: tuple[tuple[str, str], ...]) -> tuple[tuple
def mask_local_paths_in_output(output: str, thread_data: ThreadDataState | None) -> str:
"""Mask host absolute paths from local sandbox output using virtual paths.
- Handles user-data paths (per-thread), skills paths, and ACP workspace paths (global).
+ Handles user-data paths (per-thread), skills paths (global + per-user
+ custom), and ACP workspace paths (per-thread).
"""
# Build the ordered (host_base, virtual_base) source list. Order is
- # preserved from the original implementation: skills, then ACP workspace,
- # then user-data mappings (longest host path first). Custom mount host
- # paths are masked by LocalSandbox._reverse_resolve_paths_in_output().
+ # preserved from the original implementation: skills, then per-user
+ # custom skills, then ACP workspace, then user-data mappings (longest
+ # host path first). Custom mount host paths are masked by
+ # LocalSandbox._reverse_resolve_paths_in_output().
sources: list[tuple[str, str]] = []
skills_host = _get_skills_host_path()
if skills_host:
sources.append((skills_host, _get_skills_container_path()))
+ # Per-user custom skills: mask host paths under the user's custom
+ # skills directory back to /mnt/skills/custom. The sandbox's
+ # _reverse_resolve_path handles this for its own operations, but
+ # mask_local_paths_in_output serves as a safety net for edge cases
+ # where host paths appear in output that bypassed sandbox resolution.
+ try:
+ from deerflow.config.paths import get_paths
+ from deerflow.runtime.user_context import get_effective_user_id
+
+ user_id = get_effective_user_id()
+ user_custom_dir = get_paths().user_custom_skills_dir(user_id)
+ if user_custom_dir.exists():
+ skills_container = _get_skills_container_path()
+ sources.append((str(user_custom_dir), f"{skills_container}/custom"))
+ except Exception:
+ pass
+
acp_host = _get_acp_workspace_host_path(_extract_thread_id_from_thread_data(thread_data))
if acp_host:
sources.append((acp_host, _ACP_WORKSPACE_VIRTUAL_PATH))
@@ -1032,40 +1172,26 @@ def validate_local_bash_command_paths(command: str, thread_data: ThreadDataState
def replace_virtual_paths_in_command(command: str, thread_data: ThreadDataState | None) -> str:
- """Replace all virtual paths (/mnt/user-data, /mnt/skills, /mnt/acp-workspace) in a command string.
+ """Replace /mnt/user-data virtual paths in a command string for local sandbox.
+
+ Skills paths (/mnt/skills) and ACP workspace paths (/mnt/acp-workspace)
+ are NOT replaced here — LocalSandbox._resolve_paths_in_command() resolves
+ them via PathMapping at execution time, which uses the correct user_id
+ from sandbox acquire. Pre-resolving with _resolve_skills_path /
+ _resolve_acp_workspace_path uses get_effective_user_id() from contextvar
+ which may differ from the sandbox mapping's user_id.
Args:
command: The command string that may contain virtual paths.
thread_data: The thread data containing actual paths.
Returns:
- The command with all virtual paths replaced.
+ The command with user-data virtual paths replaced.
"""
result = command
- # Replace skills paths
- skills_container = _get_skills_container_path()
- skills_host = _get_skills_host_path()
- if skills_host and skills_container in result:
- skills_pattern = re.compile(rf"{re.escape(skills_container)}(/[^\s\"';&|<>()]*)?")
-
- def replace_skills_match(match: re.Match) -> str:
- return _resolve_skills_path(match.group(0)).replace("\\", "/")
-
- result = skills_pattern.sub(replace_skills_match, result)
-
- # Replace ACP workspace paths
- _thread_id = _extract_thread_id_from_thread_data(thread_data)
- acp_host = _get_acp_workspace_host_path(_thread_id)
- if acp_host and _ACP_WORKSPACE_VIRTUAL_PATH in result:
- acp_pattern = re.compile(rf"{re.escape(_ACP_WORKSPACE_VIRTUAL_PATH)}(/[^\s\"';&|<>()]*)?")
-
- def replace_acp_match(match: re.Match, _tid: str | None = _thread_id) -> str:
- return _resolve_acp_workspace_path(match.group(0), _tid).replace("\\", "/")
-
- result = acp_pattern.sub(replace_acp_match, result)
-
- # Custom mount paths are resolved by LocalSandbox._resolve_paths_in_command()
+ # Skills, ACP workspace, and custom mount paths are resolved by
+ # LocalSandbox._resolve_paths_in_command() via PathMapping.
# Replace user-data paths
if VIRTUAL_PATH_PREFIX in result and thread_data is not None:
@@ -1495,6 +1621,10 @@ def ls_tool(runtime: Runtime, description: str, path: str) -> str:
path: The **absolute** path to the directory to list.
"""
try:
+ # Block access to disabled skill directories
+ if _is_disabled_skill_path(path, user_id=resolve_runtime_user_id(runtime)):
+ skill_name = _extract_skill_name_from_skills_path(path) or "unknown"
+ return f"Error: Skill '{skill_name}' is disabled. Access to its files is blocked. Enable the skill in settings before using it."
sandbox = ensure_sandbox_initialized(runtime)
ensure_thread_directories_exist(runtime)
requested_path = path
@@ -1502,13 +1632,16 @@ def ls_tool(runtime: Runtime, description: str, path: str) -> str:
if is_local_sandbox(runtime):
thread_data = get_thread_data(runtime)
validate_local_tool_path(path, thread_data, read_only=True)
- if _is_skills_path(path):
- path = _resolve_skills_path(path)
- elif _is_acp_workspace_path(path):
- path = _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data))
+ if _is_skills_path(path) or _is_acp_workspace_path(path):
+ # Skills and ACP workspace paths are resolved by the sandbox's
+ # PathMapping (which uses the user_id from acquire time), not
+ # by _resolve_skills_path / _resolve_acp_workspace_path (which
+ # use get_effective_user_id() from contextvar and may differ
+ # from the sandbox mapping's user_id).
+ pass
elif not _is_custom_mount_path(path):
path = _resolve_and_validate_user_data_path(path, thread_data)
- # Custom mount paths are resolved by LocalSandbox._resolve_path()
+ # Custom mount paths and skills/ACP paths are resolved by LocalSandbox._resolve_path()
children = sandbox.list_dir(path)
if not children:
return "(empty)"
@@ -1748,6 +1881,10 @@ def read_file_tool(
end_line: Optional ending line number (1-indexed, inclusive). Use with start_line to read a specific range.
"""
try:
+ # Block access to disabled skill files
+ if _is_disabled_skill_path(path, user_id=resolve_runtime_user_id(runtime)):
+ skill_name = _extract_skill_name_from_skills_path(path) or "unknown"
+ return f"Error: Skill '{skill_name}' is disabled. Access to its files is blocked. Enable the skill in settings before using it."
requested_path = path
content = read_current_file_content(runtime, path)
if not content:
diff --git a/backend/packages/harness/deerflow/skills/storage/__init__.py b/backend/packages/harness/deerflow/skills/storage/__init__.py
index c4488b1a6..f91a1ace0 100644
--- a/backend/packages/harness/deerflow/skills/storage/__init__.py
+++ b/backend/packages/harness/deerflow/skills/storage/__init__.py
@@ -5,15 +5,32 @@ Mirrors the pattern used by ``deerflow/sandbox/sandbox_provider.py``.
from __future__ import annotations
+import logging
import threading
+from collections import OrderedDict
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
from deerflow.skills.storage.skill_storage import SkillStorage
+from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
+
+logger = logging.getLogger(__name__)
_default_skill_storage: SkillStorage | None = None
_default_skill_storage_config: object | None = None # AppConfig identity the singleton was built from
_skill_storage_lock = threading.Lock()
+# Maximum number of per-user storage instances to keep in cache.
+# Real-world deployments rarely have more than a few concurrent users per
+# process; 64 is a generous ceiling that prevents unbounded memory growth.
+_MAX_USER_SCOPED_STORAGES = 64
+
+# Per-user skill storage cache with double-check lock for concurrent creation.
+# OrderedDict so that LRU eviction can remove the least-recently-used entry
+# via ``move_to_end`` + ``popitem(last=False)`` when the cache exceeds
+# ``_MAX_USER_SCOPED_STORAGES``.
+_user_scoped_storages: OrderedDict[str, UserScopedSkillStorage] = OrderedDict()
+_user_scoped_storage_lock = threading.Lock()
+
def get_or_new_skill_storage(**kwargs) -> SkillStorage:
"""Return a ``SkillStorage`` instance — either a new one or the process singleton.
@@ -27,6 +44,10 @@ def get_or_new_skill_storage(**kwargs) -> SkillStorage:
**Singleton** is returned (created on first call, then reused) when neither
``skills_path`` nor ``app_config`` is given — uses ``get_app_config()`` to
resolve the active configuration.
+
+ This singleton is used for reading **public** skills (global, read-only).
+ For user-scoped custom skill operations, use
+ :func:`get_or_new_user_skill_storage` instead.
"""
global _default_skill_storage, _default_skill_storage_config
@@ -79,17 +100,86 @@ def get_or_new_skill_storage(**kwargs) -> SkillStorage:
return _default_skill_storage
+def get_or_new_user_skill_storage(user_id: str, **kwargs) -> SkillStorage:
+ """Return a per-user ``SkillStorage`` instance for custom skill isolation.
+
+ Uses :class:`UserScopedSkillStorage` which redirects custom skill paths
+ to ``{base_dir}/users/{user_id}/skills/custom/`` while keeping public
+ skill reads from the global root.
+
+ ``user_id`` is normalised via :func:`make_safe_user_id` so that external
+ identities (e.g. IM channel ids containing non-``[A-Za-z0-9_-]`` chars)
+ are safely bucketed before reaching :class:`UserScopedSkillStorage`, which
+ calls :func:`_validate_user_id` internally.
+
+ Instances are cached by the *normalised* ``user_id`` with double-check
+ locking to prevent concurrent creation races. When the cache exceeds
+ ``_MAX_USER_SCOPED_STORAGES``, the least-recently-accessed entry is
+ evicted (true LRU, not FIFO).
+ """
+ from deerflow.config.paths import make_safe_user_id
+
+ safe_id = make_safe_user_id(user_id)
+
+ # Always acquire lock so move_to_end is safe — makes this a true LRU
+ # cache instead of FIFO. The overhead is negligible since dict ops are
+ # fast and this function is called once per agent-creation cycle.
+ with _user_scoped_storage_lock:
+ cached = _user_scoped_storages.get(safe_id)
+ if cached is not None:
+ _user_scoped_storages.move_to_end(safe_id)
+ return cached
+
+ cached = UserScopedSkillStorage(safe_id, **kwargs)
+ _user_scoped_storages[safe_id] = cached
+ # Evict least-recently-used entry if cache exceeds the ceiling.
+ # Since we just moved the current user_id to the end, popitem(last=False)
+ # will evict the oldest/least-recently-accessed entry (never the
+ # one we just created).
+ while len(_user_scoped_storages) > _MAX_USER_SCOPED_STORAGES:
+ evicted_key, evicted_val = _user_scoped_storages.popitem(last=False)
+ logger.info("Evicted user-scoped skill storage for safe_id=%s (cache ceiling %d)", evicted_key, _MAX_USER_SCOPED_STORAGES)
+ return cached
+
+
def reset_skill_storage() -> None:
- """Clear the cached singleton (used in tests and hot-reload scenarios)."""
+ """Clear all cached storage instances (used in tests and hot-reload scenarios)."""
global _default_skill_storage, _default_skill_storage_config
with _skill_storage_lock:
_default_skill_storage = None
_default_skill_storage_config = None
+ with _user_scoped_storage_lock:
+ _user_scoped_storages.clear()
+
+
+def reset_user_skill_storage(user_id: str | None = None) -> None:
+ """Clear per-user skill storage cache for a specific user, or all users.
+
+ ``user_id`` is normalised via :func:`make_safe_user_id` so that the
+ cache key matches the one used by :func:`get_or_new_user_skill_storage`.
+ Without normalisation, IM-channel user IDs (e.g. ``feishu:xxx``) would
+ fail to clear their stale cache entries.
+
+ Args:
+ user_id: If provided, remove only that user's cached storage.
+ If ``None``, clear the entire per-user cache.
+ """
+ from deerflow.config.paths import make_safe_user_id
+
+ with _user_scoped_storage_lock:
+ if user_id is not None:
+ safe_id = make_safe_user_id(user_id)
+ _user_scoped_storages.pop(safe_id, None)
+ else:
+ _user_scoped_storages.clear()
__all__ = [
"LocalSkillStorage",
"SkillStorage",
+ "UserScopedSkillStorage",
"get_or_new_skill_storage",
+ "get_or_new_user_skill_storage",
"reset_skill_storage",
+ "reset_user_skill_storage",
]
diff --git a/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py b/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py
index cbb5f5c14..67a1b03e3 100644
--- a/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py
+++ b/backend/packages/harness/deerflow/skills/storage/local_skill_storage.py
@@ -189,6 +189,7 @@ class LocalSkillStorage(SkillStorage):
staging_target = Path(staging_root) / skill_name
shutil.copytree(skill_dir, staging_target)
_move_staged_skill_into_reserved_target(staging_target, target)
+ make_skill_written_path_sandbox_readable(custom_dir, target)
def delete_custom_skill(self, name: str, *, history_meta: dict | None = None) -> None:
self.validate_skill_name(name)
diff --git a/backend/packages/harness/deerflow/skills/storage/skill_storage.py b/backend/packages/harness/deerflow/skills/storage/skill_storage.py
index 7ebb0987e..08138e3ce 100644
--- a/backend/packages/harness/deerflow/skills/storage/skill_storage.py
+++ b/backend/packages/harness/deerflow/skills/storage/skill_storage.py
@@ -110,6 +110,28 @@ class SkillStorage(ABC):
Origin: ``deerflow.skills.loader.get_skills_root_path``.
"""
+ def validate_skill_file_path(self, skill_file: Path) -> Path:
+ """Validate that *skill_file* is within an allowed root and return its resolved path.
+
+ The default implementation checks that ``skill_file`` is under
+ ``get_skills_root_path()`` — sufficient for :class:`LocalSkillStorage`
+ where both public and custom skills live under the same root.
+
+ :class:`UserScopedSkillStorage` overrides this to also accept files
+ under the per-user custom root, because custom skills are stored in a
+ separate directory tree that is not a sub-path of the global root.
+
+ Raises:
+ ValueError: if the resolved path escapes all allowed roots.
+ """
+ resolved_file = skill_file.resolve()
+ resolved_root = self.get_skills_root_path().resolve()
+ try:
+ resolved_file.relative_to(resolved_root)
+ except ValueError as exc:
+ raise ValueError("Resolved skill file must stay within the configured skills root.") from exc
+ return resolved_file
+
@abstractmethod
def _iter_skill_files(self) -> Iterable[tuple[SkillCategory, Path, Path]]:
"""Yield ``(category, category_root, skill_md_path)`` for every SKILL.md.
@@ -201,6 +223,12 @@ class SkillStorage(ABC):
def get_skill_history_file(self, name: str) -> Path:
"""Path to ``custom/.history/.jsonl``. Does not create parents.
+ **Note:** This default implementation returns a path under the global
+ skills root, which is correct for :class:`LocalSkillStorage` but
+ **incorrect** for :class:`UserScopedSkillStorage`. Subclasses that
+ redirect custom skill paths must override this method (as
+ ``UserScopedSkillStorage`` already does).
+
Origin: ``deerflow.skills.manager.get_skill_history_file``.
"""
normalized_name = self.validate_skill_name(name)
@@ -231,6 +259,10 @@ class SkillStorage(ABC):
# Merge enabled state from extensions config (re-read every call so
# changes made by another process are picked up immediately).
+ # All skill categories (PUBLIC, LEGACY, CUSTOM) respect the
+ # extensions_config enabled/disabled state. CUSTOM skills default
+ # to enabled when no explicit config entry exists (so newly
+ # installed skills appear active without requiring a manual toggle).
try:
from deerflow.config.extensions_config import ExtensionsConfig
@@ -247,7 +279,12 @@ class SkillStorage(ABC):
return skills
def ensure_custom_skill_is_editable(self, name: str) -> None:
- """Origin: ``deerflow.skills.manager.ensure_custom_skill_is_editable``."""
+ """Origin: ``deerflow.skills.manager.ensure_custom_skill_is_editable``.
+
+ Only CUSTOM-category skills are editable. PUBLIC (built-in) and
+ LEGACY (shared pre-migration) skills are read-only; attempting to
+ edit them raises ``ValueError`` with a helpful suggestion.
+ """
if self.custom_skill_exists(name):
return
if self.public_skill_exists(name):
diff --git a/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py
new file mode 100644
index 000000000..de56d1f31
--- /dev/null
+++ b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py
@@ -0,0 +1,387 @@
+"""User-scoped SkillStorage that isolates custom skills per user.
+
+Custom skills are stored under ``{base_dir}/users/{user_id}/skills/custom/``
+instead of the global ``{base_dir}/skills/custom/``. Public skills are still
+read from the global ``{base_dir}/skills/public/`` (read-only).
+
+Layout::
+
+ /public//SKILL.md ← global, read-only
+ //SKILL.md ← per-user, read-write
+ /.history/.jsonl ← per-user history
+ /_skill_states.json ← per-user enabled state
+ //SKILL.md ← legacy fallback, read-only
+
+Fallback: when a user has no custom skills yet, global ``skills/custom/``
+skills are yielded as ``SkillCategory.LEGACY`` (read-only) so they are
+visible but cannot be edited/deleted by the user. This preserves backward
+compatibility during migration without leaking mutable access to legacy
+skills. Legacy skills are mounted at ``/mnt/skills/legacy//`` in
+the sandbox so their supporting files (references, templates, scripts,
+assets) are accessible to the agent.
+
+Enabled/disabled state for CUSTOM and LEGACY skills is stored per-user in
+``_skill_states.json`` (keyed by skill name). PUBLIC skill state remains
+global in ``extensions_config.json``. This prevents cross-user bleed when
+two users own same-named custom skills.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import json
+import logging
+import os
+import tempfile
+from collections.abc import Iterable
+from pathlib import Path
+
+from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
+from deerflow.skills.permissions import make_skill_written_path_sandbox_readable
+from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
+from deerflow.skills.storage.skill_storage import SKILL_MD_FILE
+from deerflow.skills.types import SkillCategory
+
+logger = logging.getLogger(__name__)
+
+
+class UserScopedSkillStorage(LocalSkillStorage):
+ """Skill storage with per-user isolation for custom skills.
+
+ Inherits all public-skill behaviour from :class:`LocalSkillStorage`
+ (reading from ``_host_root/public/``). Custom-skill paths are
+ redirected to ``_user_custom_root`` so each user's custom skills
+ live in their own directory tree.
+
+ Fallback: when the user's custom directory is empty and the global
+ ``skills/custom/`` has content, those legacy skills are loaded as
+ ``SkillCategory.LEGACY`` — they appear in listings but are treated
+ as read-only (cannot be edited/deleted). This preserves backward
+ compatibility during migration without giving users mutable access
+ to other users' legacy skills.
+
+ **Design note**: once a user creates their first custom skill, the
+ per-user directory exists and the global custom fallback no longer
+ applies — LEGACY skills disappear from that user's listing. This is
+ intentional (shadow-mount semantics: the user's own directory
+ shadows the global one).
+ """
+
+ def __init__(
+ self,
+ user_id: str,
+ host_path: str | None = None,
+ container_path: str = DEFAULT_SKILLS_CONTAINER_PATH,
+ app_config=None,
+ ) -> None:
+ super().__init__(host_path=host_path, container_path=container_path, app_config=app_config)
+
+ from deerflow.config.paths import _validate_user_id, get_paths
+
+ self._user_id = _validate_user_id(user_id)
+ paths = get_paths()
+ self._user_custom_root: Path = paths.user_custom_skills_dir(self._user_id)
+ self._user_skills_root: Path = paths.user_skills_dir(self._user_id)
+ self._global_custom_root: Path = self._host_root / SkillCategory.CUSTOM.value
+ self._skill_states_file: Path = self._user_skills_root / "_skill_states.json"
+
+ # ------------------------------------------------------------------
+ # Per-user skill enabled state (CUSTOM / LEGACY only)
+ # ------------------------------------------------------------------
+
+ def _read_skill_states(self) -> dict[str, dict[str, bool]]:
+ """Read per-user skill enabled states from ``_skill_states.json``.
+
+ Returns a dict keyed by skill name, each value being
+ ``{"enabled": True/False}``. Returns an empty dict if the file
+ does not exist or is unreadable.
+ """
+ if not self._skill_states_file.exists():
+ return {}
+ try:
+ with open(self._skill_states_file, encoding="utf-8") as f:
+ data = json.load(f)
+ if isinstance(data, dict):
+ return data
+ except (json.JSONDecodeError, OSError):
+ logger.warning("Failed to read skill states file %s", self._skill_states_file)
+ return {}
+
+ def _write_skill_states(self, states: dict[str, dict[str, bool]]) -> None:
+ """Persist per-user skill enabled states to ``_skill_states.json``.
+
+ Atomic write via a temp file in the same directory followed by
+ ``Path.replace`` (POSIX-atomic on the same filesystem). Without this,
+ a crash/SIGTERM/disk-full mid-write would leave the file truncated
+ or empty; ``_read_skill_states`` would then return ``{}`` and
+ ``get_skill_enabled_state`` would silently re-enable every skill
+ the user had disabled. Mirrors the pattern used by
+ ``LocalSkillStorage.write_custom_skill`` in this same module.
+ """
+ self._user_skills_root.mkdir(parents=True, exist_ok=True)
+ fd, tmp_path_str = tempfile.mkstemp(
+ dir=str(self._user_skills_root),
+ prefix=".skill_states_",
+ suffix=".json.tmp",
+ )
+ tmp_path = Path(tmp_path_str)
+ try:
+ with os.fdopen(fd, "w", encoding="utf-8") as f:
+ json.dump(states, f, indent=2)
+ tmp_path.replace(self._skill_states_file)
+ except Exception:
+ # Best-effort cleanup of the temp file on failure.
+ try:
+ tmp_path.unlink(missing_ok=True)
+ except OSError:
+ pass
+ raise
+
+ def get_skill_enabled_state(self, skill_name: str) -> bool:
+ """Return the enabled state for a custom/legacy skill.
+
+ Default is ``True`` (newly created skills are enabled by default).
+ """
+ states = self._read_skill_states()
+ entry = states.get(skill_name)
+ if entry is None:
+ return True
+ return entry.get("enabled", True)
+
+ def set_skill_enabled_state(self, skill_name: str, enabled: bool) -> None:
+ """Set the enabled state for a custom/legacy skill and persist."""
+ states = self._read_skill_states()
+ states[skill_name] = {"enabled": enabled}
+ self._write_skill_states(states)
+
+ # ------------------------------------------------------------------
+ # Path helpers — redirect custom skill paths to user directory
+ # ------------------------------------------------------------------
+
+ def get_custom_skill_dir(self, name: str) -> Path:
+ """Per-user custom skill directory: ``//``."""
+ normalized_name = self.validate_skill_name(name)
+ return self._user_custom_root / normalized_name
+
+ def get_custom_skill_file(self, name: str) -> Path:
+ """Per-user custom SKILL.md path."""
+ return self.get_custom_skill_dir(name) / SKILL_MD_FILE
+
+ def get_skill_history_file(self, name: str) -> Path:
+ """Per-user custom skill history: ``/.history/.jsonl``."""
+ normalized_name = self.validate_skill_name(name)
+ return self._user_custom_root / ".history" / f"{normalized_name}.jsonl"
+
+ # ------------------------------------------------------------------
+ # Enabled state — override to use per-user state for custom/legacy
+ # ------------------------------------------------------------------
+
+ def load_skills(self, *, enabled_only: bool = False) -> list:
+ """Discover all skills and merge enabled state per isolation scope.
+
+ Delegates skill discovery and PUBLIC enabled-state to
+ :meth:`LocalSkillStorage.load_skills` (which reads from the
+ overridden ``_iter_skill_files``). Then overrides CUSTOM/LEGACY
+ enabled state with per-user ``_skill_states.json`` so that two
+ users each owning a same-named custom skill can toggle independently.
+
+ Calling ``super().load_skills()`` preserves the full template-method
+ flow (discover → global enabled-state merge → filter → sort) so
+ that patching ``LocalSkillStorage.load_skills`` in tests still
+ intercepts the call.
+ """
+ # Let the parent do full discovery + global enabled-state merge.
+ # The overridden _iter_skill_files() routes custom reads to
+ # _user_custom_root and legacy reads to _global_custom_root.
+ skills = super().load_skills(enabled_only=False)
+
+ # Override enabled state for CUSTOM / LEGACY with per-user state,
+ # ANDed with the global extensions_config default. This preserves a
+ # pre-upgrade global disable of a shared custom/legacy skill from
+ # being silently re-enabled by an absent per-user entry, while still
+ # letting the per-user state override the global default when both
+ # are present. PUBLIC skill state remains governed solely by
+ # extensions_config (handled by ``super().load_skills`` above).
+ from deerflow.config.extensions_config import get_extensions_config
+
+ extensions_config = get_extensions_config()
+ for skill in skills:
+ category = skill.category.value if hasattr(skill.category, "value") else skill.category
+ if category != SkillCategory.PUBLIC.value:
+ per_user_state = self.get_skill_enabled_state(skill.name)
+ global_state = extensions_config.is_skill_enabled(skill.name, category)
+ skill.enabled = per_user_state and global_state
+
+ if enabled_only:
+ skills = [s for s in skills if s.enabled]
+
+ return skills
+
+ # ------------------------------------------------------------------
+ # Skill iteration — public from global, custom from user dir + fallback
+ # ------------------------------------------------------------------
+
+ def public_skill_exists(self, name: str) -> bool:
+ """Check if a skill exists as public **or** as a global-custom fallback.
+
+ The global ``skills/custom/`` directory contains legacy skills that
+ are presented as ``SkillCategory.LEGACY`` to users who have no
+ per-user custom skills yet. This override ensures those skills are
+ recognised as "read-only" so ``ensure_custom_skill_is_editable``
+ can give a helpful error message instead of ``FileNotFoundError``.
+ """
+ normalized_name = self.validate_skill_name(name)
+ # Standard public check
+ if (self._host_root / SkillCategory.PUBLIC.value / normalized_name / SKILL_MD_FILE).exists():
+ return True
+ # Global custom fallback check (legacy skills visible to all users)
+ if (self._global_custom_root / normalized_name / SKILL_MD_FILE).exists():
+ return True
+ return False
+
+ def ensure_custom_skill_is_editable(self, name: str) -> None:
+ """Override to handle global-custom fallback skills gracefully.
+
+ When a user tries to edit/delete a legacy global-custom skill (one
+ that appears as ``SkillCategory.LEGACY`` due to fallback), we tell
+ them to create their own version rather than raising a confusing
+ ``FileNotFoundError``.
+ """
+ if self.custom_skill_exists(name):
+ return
+ # Check both public and global-custom fallback
+ normalized_name = self.validate_skill_name(name)
+ is_global_public = (self._host_root / SkillCategory.PUBLIC.value / normalized_name / SKILL_MD_FILE).exists()
+ is_global_custom_fallback = (self._global_custom_root / normalized_name / SKILL_MD_FILE).exists()
+ if is_global_public:
+ raise ValueError(f"'{name}' is a built-in skill. Use the skill_manage tool to create your own version — it will shadow the built-in one.")
+ if is_global_custom_fallback:
+ raise ValueError(f"'{name}' is a legacy shared skill (not editable). To customise it, create your own version with the same name — it will shadow the shared one.")
+ raise FileNotFoundError(f"Custom skill '{name}' not found.")
+
+ def _iter_skill_files(self) -> Iterable[tuple[SkillCategory, Path, Path]]:
+ # 1. Public skills: always from global root
+ public_path = self._host_root / SkillCategory.PUBLIC.value
+ if public_path.exists() and public_path.is_dir():
+ for current_root, dir_names, file_names in os.walk(public_path, followlinks=True):
+ dir_names[:] = sorted(name for name in dir_names if not name.startswith("."))
+ if SKILL_MD_FILE not in file_names:
+ continue
+ yield SkillCategory.PUBLIC, public_path, Path(current_root) / SKILL_MD_FILE
+
+ # 2. Custom skills: prefer user-level directory
+ user_custom_exists = False
+ user_custom_path = self._user_custom_root
+ if user_custom_path.exists() and user_custom_path.is_dir():
+ for current_root, dir_names, file_names in os.walk(user_custom_path, followlinks=True):
+ dir_names[:] = sorted(name for name in dir_names if not name.startswith(".") and name != ".history")
+ if SKILL_MD_FILE not in file_names:
+ continue
+ user_custom_exists = True
+ yield SkillCategory.CUSTOM, user_custom_path, Path(current_root) / SKILL_MD_FILE
+
+ # 3. Fallback: if user has no custom skills, load from global custom
+ # as LEGACY (read-only) so legacy skills are visible but not
+ # editable/deletable by the user. LEGACY skills are mounted at
+ # /mnt/skills/legacy// in the sandbox so their supporting
+ # files (references, templates, scripts, assets) are accessible.
+ if not user_custom_exists:
+ global_custom_path = self._global_custom_root
+ if global_custom_path.exists() and global_custom_path.is_dir():
+ for current_root, dir_names, file_names in os.walk(global_custom_path, followlinks=True):
+ dir_names[:] = sorted(name for name in dir_names if not name.startswith(".") and name != ".history")
+ if SKILL_MD_FILE not in file_names:
+ continue
+ yield SkillCategory.LEGACY, global_custom_path, Path(current_root) / SKILL_MD_FILE
+
+ # ------------------------------------------------------------------
+ # Install — redirect custom_dir to user directory
+ # ------------------------------------------------------------------
+
+ async def ainstall_skill_from_archive(self, archive_path: str | Path) -> dict:
+ from deerflow.skills.installer import _scan_skill_archive_contents_or_raise
+
+ logger.info("Installing skill from %s for user %s", archive_path, self._user_id)
+ path = Path(archive_path)
+ custom_dir = self._user_custom_root
+
+ # Ensure user custom directory exists
+ custom_dir.mkdir(parents=True, exist_ok=True)
+
+ # The per-file security scan is an async LLM call and must stay on the
+ # event loop; every filesystem phase around it runs in a worker thread.
+ tmp = await asyncio.to_thread(tempfile.mkdtemp)
+ try:
+ skill_dir, skill_name, target = await asyncio.to_thread(self._prepare_skill_archive, path, Path(tmp), custom_dir, archive_path)
+
+ await _scan_skill_archive_contents_or_raise(skill_dir, skill_name)
+
+ await asyncio.to_thread(self._commit_skill_install, skill_dir, skill_name, custom_dir, target)
+ logger.info("Skill %r installed to %s for user %s", skill_name, target, self._user_id)
+ finally:
+ try:
+ await asyncio.wait_for(
+ asyncio.to_thread(self._cleanup_install_tmp, tmp),
+ timeout=5.0,
+ )
+ except TimeoutError:
+ logger.warning("Timed out cleaning up skill install temp dir %s", tmp)
+
+ return {
+ "success": True,
+ "skill_name": skill_name,
+ "message": f"Skill '{skill_name}' installed successfully for user '{self._user_id}'",
+ }
+
+ # ------------------------------------------------------------------
+ # Write — ensure user custom dir exists before writing
+ # ------------------------------------------------------------------
+
+ def write_custom_skill(self, name: str, relative_path: str, content: str) -> None:
+ # Ensure user custom skills directory exists
+ self._user_custom_root.mkdir(parents=True, exist_ok=True)
+ target = self.validate_relative_path(relative_path, self.get_custom_skill_dir(name))
+ target.parent.mkdir(parents=True, exist_ok=True)
+ with tempfile.NamedTemporaryFile(
+ "w",
+ encoding="utf-8",
+ delete=False,
+ dir=str(target.parent),
+ ) as tmp_file:
+ tmp_file.write(content)
+ tmp_path = Path(tmp_file.name)
+ tmp_path.replace(target)
+ make_skill_written_path_sandbox_readable(self.get_custom_skill_dir(name), target)
+
+ # ------------------------------------------------------------------
+ # Public helpers
+ # ------------------------------------------------------------------
+
+ @property
+ def user_id(self) -> str:
+ """The user ID this storage is scoped to."""
+ return self._user_id
+
+ def get_user_custom_root(self) -> Path:
+ """Host path to this user's custom skills root directory."""
+ return self._user_custom_root
+
+ # ------------------------------------------------------------------
+ # Path validation — accept per-user custom root as well as global root
+ # ------------------------------------------------------------------
+
+ def validate_skill_file_path(self, skill_file: Path) -> Path:
+ """Accept files under *either* the global root or the per-user custom root.
+
+ Custom skills live in ``_user_custom_root`` which is not a sub-path
+ of ``_host_root``, so the default implementation's single-root check
+ would reject them. This override allows both roots.
+ """
+ resolved_file = skill_file.resolve()
+ for allowed_root in (self._host_root.resolve(), self._user_custom_root.resolve()):
+ try:
+ resolved_file.relative_to(allowed_root)
+ return resolved_file
+ except ValueError:
+ continue
+ raise ValueError(f"Resolved skill file {resolved_file} must stay within either the global skills root ({self._host_root.resolve()}) or the per-user custom root ({self._user_custom_root.resolve()}).")
diff --git a/backend/packages/harness/deerflow/skills/types.py b/backend/packages/harness/deerflow/skills/types.py
index 4cb3ade74..5d607bf9d 100644
--- a/backend/packages/harness/deerflow/skills/types.py
+++ b/backend/packages/harness/deerflow/skills/types.py
@@ -12,10 +12,14 @@ class SkillCategory(StrEnum):
- ``PUBLIC``: built-in skill bundled with the platform, read-only.
- ``CUSTOM``: user-authored skill that can be edited or deleted.
+ - ``LEGACY``: global custom skill from before user-isolation migration,
+ presented as read-only (visible but not editable/deletable). These
+ skills are mounted at ``/mnt/skills/legacy//`` in the sandbox.
"""
PUBLIC = "public"
CUSTOM = "custom"
+ LEGACY = "legacy"
@dataclass(frozen=True)
diff --git a/backend/packages/harness/deerflow/tools/skill_manage_tool.py b/backend/packages/harness/deerflow/tools/skill_manage_tool.py
index 2a39732bc..97091e719 100644
--- a/backend/packages/harness/deerflow/tools/skill_manage_tool.py
+++ b/backend/packages/harness/deerflow/tools/skill_manage_tool.py
@@ -9,9 +9,10 @@ from weakref import WeakValueDictionary
from langchain.tools import tool
-from deerflow.agents.lead_agent.prompt import refresh_skills_system_prompt_cache_async
+from deerflow.agents.lead_agent.prompt import refresh_user_skills_system_prompt_cache_async
+from deerflow.runtime.user_context import resolve_runtime_user_id
from deerflow.skills.security_scanner import scan_skill_content
-from deerflow.skills.storage import get_or_new_skill_storage
+from deerflow.skills.storage import get_or_new_user_skill_storage
from deerflow.skills.storage.skill_storage import SkillStorage
from deerflow.skills.types import SKILL_MD_FILE
from deerflow.tools.sync import make_sync_tool_wrapper
@@ -19,14 +20,16 @@ from deerflow.tools.types import Runtime
logger = logging.getLogger(__name__)
-_skill_locks: WeakValueDictionary[str, asyncio.Lock] = WeakValueDictionary()
+# Lock granularity: (user_id, skill_name) to avoid cross-user blocking.
+_skill_locks: WeakValueDictionary[tuple[str, str], asyncio.Lock] = WeakValueDictionary()
-def _get_lock(name: str) -> asyncio.Lock:
- lock = _skill_locks.get(name)
+def _get_lock(user_id: str, name: str) -> asyncio.Lock:
+ key = (user_id, name)
+ lock = _skill_locks.get(key)
if lock is None:
lock = asyncio.Lock()
- _skill_locks[name] = lock
+ _skill_locks[key] = lock
return lock
@@ -85,9 +88,10 @@ async def _skill_manage_impl(
expected_count: Optional expected number of replacements for patch.
"""
name = SkillStorage.validate_skill_name(name)
- lock = _get_lock(name)
+ user_id = resolve_runtime_user_id(runtime)
+ lock = _get_lock(user_id, name)
thread_id = _get_thread_id(runtime)
- skill_storage = get_or_new_skill_storage()
+ skill_storage = get_or_new_user_skill_storage(user_id)
async with lock:
if action == "create":
@@ -103,9 +107,8 @@ async def _skill_manage_impl(
name,
_history_record(action="create", file_path=SKILL_MD_FILE, prev_content=None, new_content=content, thread_id=thread_id, scanner=scan),
)
- await refresh_skills_system_prompt_cache_async()
+ await refresh_user_skills_system_prompt_cache_async(user_id)
return f"Created custom skill '{name}'."
-
if action == "edit":
await _to_thread(skill_storage.ensure_custom_skill_is_editable, name)
if content is None:
@@ -120,7 +123,7 @@ async def _skill_manage_impl(
name,
_history_record(action="edit", file_path=SKILL_MD_FILE, prev_content=prev_content, new_content=content, thread_id=thread_id, scanner=scan),
)
- await refresh_skills_system_prompt_cache_async()
+ await refresh_user_skills_system_prompt_cache_async(user_id)
return f"Updated custom skill '{name}'."
if action == "patch":
@@ -144,7 +147,7 @@ async def _skill_manage_impl(
name,
_history_record(action="patch", file_path=SKILL_MD_FILE, prev_content=prev_content, new_content=new_content, thread_id=thread_id, scanner=scan),
)
- await refresh_skills_system_prompt_cache_async()
+ await refresh_user_skills_system_prompt_cache_async(user_id)
return f"Patched custom skill '{name}' ({replacement_count} replacement(s) applied, {occurrences} match(es) found)."
if action == "delete":
@@ -160,7 +163,7 @@ async def _skill_manage_impl(
scanner={"decision": "allow", "reason": "Deletion requested."},
),
)
- await refresh_skills_system_prompt_cache_async()
+ await refresh_user_skills_system_prompt_cache_async(user_id)
return f"Deleted custom skill '{name}'."
if action == "write_file":
@@ -178,6 +181,7 @@ async def _skill_manage_impl(
name,
_history_record(action="write_file", file_path=path, prev_content=prev_content, new_content=content, thread_id=thread_id, scanner=scan),
)
+ await refresh_user_skills_system_prompt_cache_async(user_id)
return f"Wrote '{path}' for custom skill '{name}'."
if action == "remove_file":
@@ -194,10 +198,14 @@ async def _skill_manage_impl(
name,
_history_record(action="remove_file", file_path=path, prev_content=prev_content, new_content=None, thread_id=thread_id, scanner={"decision": "allow", "reason": "Deletion requested."}),
)
+ await refresh_user_skills_system_prompt_cache_async(user_id)
return f"Removed '{path}' from custom skill '{name}'."
if await _to_thread(skill_storage.public_skill_exists, name):
- raise ValueError(f"'{name}' is a built-in skill. To customise it, create a new skill with the same name under skills/custom/.")
+ # public_skill_exists covers both built-in (PUBLIC) and legacy (LEGACY)
+ # skills; the UserScopedSkillStorage override distinguishes them in
+ # ensure_custom_skill_is_editable with category-specific messages.
+ raise ValueError(f"'{name}' is a read-only skill (built-in or legacy shared). To customise it, create your own version with the same name.")
raise ValueError(f"Unsupported action '{action}'.")
diff --git a/backend/scripts/migrate_user_isolation.py b/backend/scripts/migrate_user_isolation.py
index b77169580..4a7e5d120 100644
--- a/backend/scripts/migrate_user_isolation.py
+++ b/backend/scripts/migrate_user_isolation.py
@@ -1,4 +1,4 @@
-"""One-time migration: move legacy thread dirs and memory into per-user layout.
+"""One-time migration: move legacy thread dirs, memory, agents, and skills into per-user layout.
Usage:
PYTHONPATH=. python scripts/migrate_user_isolation.py [--dry-run] [--user-id USER_ID]
@@ -130,6 +130,88 @@ def migrate_agents(
return report
+def migrate_skills(
+ paths: Paths,
+ user_id: str = "default",
+ *,
+ dry_run: bool = False,
+) -> list[dict]:
+ """Move legacy global custom skills into per-user layout.
+
+ Legacy layout: ``{base_dir}/skills/custom/{name}/``
+ Per-user layout: ``{base_dir}/users/{user_id}/skills/custom/{name}/``
+
+ Pre-existing per-user custom skills take precedence: if a destination
+ already exists for a skill name, the legacy copy is moved to
+ ``{base_dir}/migration-conflicts/skills/{name}/`` for manual review.
+
+ Args:
+ paths: Paths instance.
+ user_id: Target user to receive the legacy custom skills (defaults to
+ ``"default"``, matching ``DEFAULT_USER_ID`` for no-auth setups).
+ dry_run: If True, only log what would happen.
+
+ Returns:
+ List of migration report entries, one per legacy custom skill directory found.
+ """
+ report: list[dict] = []
+ legacy_custom = paths.base_dir / "skills" / "custom"
+ if not legacy_custom.exists():
+ logger.info("No legacy skills/custom directory found — nothing to migrate.")
+ return report
+
+ dest_root = paths.user_custom_skills_dir(user_id)
+
+ # Migrate .history directory first (skill operation logs)
+ legacy_history = legacy_custom / ".history"
+ if legacy_history.exists() and legacy_history.is_dir():
+ dest_history = dest_root / ".history"
+ if dest_history.exists():
+ conflicts_history = paths.base_dir / "migration-conflicts" / "skills" / ".history"
+ logger.warning("Conflict for .history: moved legacy to %s", conflicts_history)
+ if not dry_run:
+ conflicts_history.parent.mkdir(parents=True, exist_ok=True)
+ shutil.move(str(legacy_history), str(conflicts_history))
+ else:
+ logger.info("Migrating .history -> %s", dest_history)
+ if not dry_run:
+ dest_root.mkdir(parents=True, exist_ok=True)
+ shutil.move(str(legacy_history), str(dest_history))
+
+ for skill_dir in sorted(legacy_custom.iterdir()):
+ if not skill_dir.is_dir():
+ continue
+ # Skip internal directories (.history is managed per-user now)
+ if skill_dir.name.startswith("."):
+ continue
+ skill_name = skill_dir.name
+ dest = dest_root / skill_name
+
+ entry = {"skill": skill_name, "user_id": user_id, "action": ""}
+
+ if dest.exists():
+ conflicts_dir = paths.base_dir / "migration-conflicts" / "skills" / skill_name
+ entry["action"] = f"conflict -> {conflicts_dir}"
+ if not dry_run:
+ conflicts_dir.parent.mkdir(parents=True, exist_ok=True)
+ shutil.move(str(skill_dir), str(conflicts_dir))
+ logger.warning("Conflict for skill %s: moved legacy copy to %s", skill_name, conflicts_dir)
+ else:
+ entry["action"] = f"moved -> {dest}"
+ if not dry_run:
+ dest_root.mkdir(parents=True, exist_ok=True)
+ shutil.move(str(skill_dir), str(dest))
+ logger.info("Migrated skill %s -> user %s", skill_name, user_id)
+
+ report.append(entry)
+
+ # Clean up empty legacy custom dir (keep skills/ parent — public/ is still in use)
+ if not dry_run and legacy_custom.exists() and not any(legacy_custom.iterdir()):
+ legacy_custom.rmdir()
+
+ return report
+
+
def migrate_memory(
paths: Paths,
user_id: str = "default",
@@ -209,6 +291,7 @@ def main() -> None:
report = migrate_thread_dirs(paths, owner_map, dry_run=args.dry_run)
migrate_memory(paths, user_id=args.user_id, dry_run=args.dry_run)
agent_report = migrate_agents(paths, user_id=args.user_id, dry_run=args.dry_run)
+ skill_report = migrate_skills(paths, user_id=args.user_id, dry_run=args.dry_run)
if report:
logger.info("Thread migration report:")
@@ -224,6 +307,13 @@ def main() -> None:
else:
logger.info("No agents to migrate.")
+ if skill_report:
+ logger.info("Skill migration report:")
+ for entry in skill_report:
+ logger.info(" skill=%s user=%s action=%s", entry["skill"], entry["user_id"], entry["action"])
+ else:
+ logger.info("No skills to migrate.")
+
unowned = [e for e in report if e["user_id"] == "default"]
if unowned:
logger.warning("%d thread(s) had no owner and were assigned to 'default':", len(unowned))
@@ -237,6 +327,13 @@ def main() -> None:
args.user_id,
)
+ if skill_report:
+ logger.warning(
+ "%d legacy custom skill(s) were assigned to '%s'. If those skills belonged to other users, move them manually under {base_dir}/users//skills/custom/.",
+ len(skill_report),
+ args.user_id,
+ )
+
if __name__ == "__main__":
main()
diff --git a/backend/tests/test_client.py b/backend/tests/test_client.py
index 4649ce224..e1255d7f7 100644
--- a/backend/tests/test_client.py
+++ b/backend/tests/test_client.py
@@ -20,6 +20,7 @@ from app.gateway.routers.threads import ThreadGoalResponse
from app.gateway.routers.uploads import UploadResponse
from deerflow.client import DeerFlowClient
from deerflow.config.paths import Paths
+from deerflow.skills.types import SkillCategory
from deerflow.uploads.manager import PathTraversalError
# ---------------------------------------------------------------------------
@@ -157,7 +158,9 @@ class TestConfigQueries:
def test_list_skills_enabled_only(self, client):
with patch("deerflow.skills.storage.local_skill_storage.LocalSkillStorage.load_skills", return_value=[]) as mock_load:
client.list_skills(enabled_only=True)
- mock_load.assert_called_once_with(enabled_only=True)
+ # UserScopedSkillStorage.load_skills calls super().load_skills(enabled_only=False)
+ # then filters enabled-only itself, so the parent call always uses enabled_only=False.
+ mock_load.assert_called_once_with(enabled_only=False)
def test_get_memory(self, client):
memory = {"version": "1.0", "facts": []}
@@ -1296,8 +1299,15 @@ class TestSkillsManagement:
# Pre-set agent to verify it gets invalidated
client._agent = MagicMock()
+ # ``update_skill`` reads the skill list twice (find + reload) and
+ # ``UserScopedSkillStorage.load_skills`` internally calls
+ # ``super().load_skills()`` once per outer call, so the patched
+ # method is invoked 4 times: provide 4 return values.
with (
- patch("deerflow.skills.storage.local_skill_storage.LocalSkillStorage.load_skills", side_effect=[[skill], [updated_skill]]),
+ patch(
+ "deerflow.skills.storage.local_skill_storage.LocalSkillStorage.load_skills",
+ side_effect=[[skill], [skill], [updated_skill], [updated_skill]],
+ ),
patch("deerflow.client.ExtensionsConfig.resolve_config_path", return_value=tmp_path),
patch("deerflow.client.get_extensions_config", return_value=ext_config),
patch("deerflow.client.reload_extensions_config"),
@@ -1331,7 +1341,11 @@ class TestSkillsManagement:
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
- with patch("deerflow.skills.storage._default_skill_storage", LocalSkillStorage(host_path=str(skills_root))):
+ local_storage = LocalSkillStorage(host_path=str(skills_root))
+ with (
+ patch("deerflow.skills.storage._default_skill_storage", local_storage),
+ patch("deerflow.client.get_or_new_user_skill_storage", lambda user_id, **kwargs: local_storage),
+ ):
result = client.install_skill(archive_path)
assert result["success"] is True
@@ -2175,7 +2189,11 @@ class TestScenarioSkillInstallAndUse:
# Step 1: Install
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
- with patch("deerflow.skills.storage._default_skill_storage", LocalSkillStorage(host_path=str(skills_root))):
+ local_storage = LocalSkillStorage(host_path=str(skills_root))
+ with (
+ patch("deerflow.skills.storage._default_skill_storage", local_storage),
+ patch("deerflow.client.get_or_new_user_skill_storage", lambda user_id, **kwargs: local_storage),
+ ):
result = client.install_skill(archive)
assert result["success"] is True
assert (skills_root / "custom" / "my-analyzer" / "SKILL.md").exists()
@@ -2415,7 +2433,11 @@ class TestGatewayConformance:
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
- with patch("deerflow.skills.storage._default_skill_storage", LocalSkillStorage(host_path=str(tmp_path))):
+ local_storage = LocalSkillStorage(host_path=str(tmp_path))
+ with (
+ patch("deerflow.skills.storage._default_skill_storage", local_storage),
+ patch("deerflow.client.get_or_new_user_skill_storage", lambda user_id, **kwargs: local_storage),
+ ):
result = client.install_skill(archive)
parsed = SkillInstallResponse(**result)
@@ -2647,7 +2669,11 @@ class TestInstallSkillSecurity:
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
- with patch("deerflow.skills.storage._default_skill_storage", LocalSkillStorage(host_path=str(skills_root))):
+ local_storage = LocalSkillStorage(host_path=str(skills_root))
+ with (
+ patch("deerflow.skills.storage._default_skill_storage", local_storage),
+ patch("deerflow.client.get_or_new_user_skill_storage", lambda user_id, **kwargs: local_storage),
+ ):
result = client.install_skill(archive)
assert result["success"] is True
@@ -2680,7 +2706,7 @@ class TestInstallSkillSecurity:
with pytest.raises(ValueError, match="Invalid skill name"):
client.install_skill(archive)
- def test_existing_skill_rejected(self, client):
+ def test_existing_skill_rejected(self, client, allow_skill_security_scan):
"""Installing a skill that already exists is rejected."""
with tempfile.TemporaryDirectory() as tmp:
tmp_path = Path(tmp)
@@ -2701,6 +2727,7 @@ class TestInstallSkillSecurity:
with (
patch("deerflow.skills.storage._default_skill_storage", LocalSkillStorage(host_path=str(skills_root))),
patch("deerflow.skills.validation._validate_skill_frontmatter", return_value=(True, "OK", "dupe-skill")),
+ patch("deerflow.client.get_or_new_user_skill_storage", return_value=LocalSkillStorage(host_path=str(skills_root))),
):
with pytest.raises(ValueError, match="already exists"):
client.install_skill(archive)
@@ -2830,6 +2857,7 @@ class TestConfigUpdateErrors:
"""FileNotFoundError when extensions_config.json cannot be located."""
skill = MagicMock()
skill.name = "some-skill"
+ skill.category = SkillCategory.PUBLIC # Only PUBLIC skills need extensions_config.json
with (
patch("deerflow.skills.storage.local_skill_storage.LocalSkillStorage.load_skills", return_value=[skill]),
diff --git a/backend/tests/test_client_e2e.py b/backend/tests/test_client_e2e.py
index 5c2b6cd9b..4a955b0a8 100644
--- a/backend/tests/test_client_e2e.py
+++ b/backend/tests/test_client_e2e.py
@@ -563,9 +563,14 @@ class TestSkillInstallation:
(skills_root / "custom").mkdir(parents=True)
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
+ local_storage = LocalSkillStorage(host_path=str(skills_root))
monkeypatch.setattr(
"deerflow.skills.storage._default_skill_storage",
- LocalSkillStorage(host_path=str(skills_root)),
+ local_storage,
+ )
+ monkeypatch.setattr(
+ "deerflow.client.get_or_new_user_skill_storage",
+ lambda user_id, **kwargs: local_storage,
)
self._skills_root = skills_root
diff --git a/backend/tests/test_internal_auth.py b/backend/tests/test_internal_auth.py
index 478b00d83..58fde00bb 100644
--- a/backend/tests/test_internal_auth.py
+++ b/backend/tests/test_internal_auth.py
@@ -48,3 +48,33 @@ def test_internal_auth_headers_can_carry_owner_user_id(monkeypatch):
finally:
monkeypatch.delenv("DEER_FLOW_INTERNAL_AUTH_TOKEN", raising=False)
importlib.reload(reloaded)
+
+
+def test_get_internal_user_normalises_unsafe_owner_user_id():
+ """P2-3: X-DeerFlow-Owner-User-Id is at the trust boundary, so the
+ synthetic internal user must use a path-safe id. ``make_safe_user_id``
+ is lossy but deterministic; two distinct raw inputs never collide.
+ """
+ import app.gateway.internal_auth as internal_auth
+ from deerflow.config.paths import make_safe_user_id
+
+ # Path-traversal-style payloads must be normalised away.
+ user_a = internal_auth.get_internal_user(owner_user_id="ou_abc/../../etc/passwd")
+ user_b = internal_auth.get_internal_user(owner_user_id="ou_abc/../../etc/passwd")
+ assert user_a.id == user_b.id
+ assert "/" not in user_a.id
+ assert ".." not in user_a.id
+
+ # Negative chat ids and unsafe punctuation must be normalised.
+ user_neg = internal_auth.get_internal_user(owner_user_id="-1001234567890:alice")
+ assert user_neg.id == make_safe_user_id("-1001234567890:alice")
+ assert ":" not in user_neg.id
+ assert user_neg.system_role == "internal"
+
+ # Already-safe ids pass through unchanged.
+ user_safe = internal_auth.get_internal_user(owner_user_id="alice_42")
+ assert user_safe.id == "alice_42"
+
+ # Empty / None falls back to default.
+ assert internal_auth.get_internal_user().id == "default"
+ assert internal_auth.get_internal_user(owner_user_id="").id == "default"
diff --git a/backend/tests/test_lead_agent_prompt.py b/backend/tests/test_lead_agent_prompt.py
index 82582acd7..647e0406a 100644
--- a/backend/tests/test_lead_agent_prompt.py
+++ b/backend/tests/test_lead_agent_prompt.py
@@ -1,4 +1,5 @@
import threading
+from pathlib import Path
from types import SimpleNamespace
from typing import cast
@@ -76,25 +77,18 @@ def test_apply_prompt_template_includes_custom_mounts(monkeypatch):
mounts = [SimpleNamespace(container_path="/home/user/shared", read_only=False)]
config = SimpleNamespace(
sandbox=SimpleNamespace(mounts=mounts),
- skills=SimpleNamespace(container_path="/mnt/skills"),
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
monkeypatch.setattr(prompt_module, "_get_enabled_skills", lambda: [])
monkeypatch.setattr(prompt_module, "get_deferred_tools_prompt_section", lambda **kwargs: "")
monkeypatch.setattr(prompt_module, "_build_acp_section", lambda **kwargs: "")
- monkeypatch.setattr(prompt_module, "_get_memory_context", lambda agent_name=None, **kwargs: "")
- monkeypatch.setattr(prompt_module, "get_agent_soul", lambda agent_name=None: "")
-
- prompt = prompt_module.apply_prompt_template()
-
- assert "`/home/user/shared`" in prompt
- assert "Custom Mounted Directories" in prompt
def test_apply_prompt_template_includes_relative_path_guidance(monkeypatch):
config = SimpleNamespace(
sandbox=SimpleNamespace(mounts=[]),
- skills=SimpleNamespace(container_path="/mnt/skills"),
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
monkeypatch.setattr(prompt_module, "_get_enabled_skills", lambda: [])
@@ -113,7 +107,7 @@ def test_apply_prompt_template_threads_explicit_app_config_without_global_config
mounts = [SimpleNamespace(container_path="/home/user/shared", read_only=False)]
explicit_config = SimpleNamespace(
sandbox=SimpleNamespace(mounts=mounts),
- skills=SimpleNamespace(container_path="/mnt/explicit-skills"),
+ skills=SimpleNamespace(container_path="/mnt/explicit-skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/explicit-skills")),
skill_evolution=SimpleNamespace(enabled=False),
tool_search=SimpleNamespace(enabled=False),
memory=SimpleNamespace(enabled=False, injection_enabled=True, max_injection_tokens=2000),
@@ -129,6 +123,7 @@ def test_apply_prompt_template_threads_explicit_app_config_without_global_config
monkeypatch.setattr("deerflow.config.get_app_config", fail_get_app_config)
monkeypatch.setattr("deerflow.config.memory_config.get_memory_config", fail_get_memory_config)
monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda app_config=None: SimpleNamespace(load_skills=lambda enabled_only=True: []))
+ monkeypatch.setattr(prompt_module, "get_or_new_user_skill_storage", lambda user_id, app_config=None: SimpleNamespace(load_skills=lambda *, enabled_only: []))
monkeypatch.setattr(prompt_module, "get_agent_soul", lambda agent_name=None: "")
prompt = prompt_module.apply_prompt_template(app_config=explicit_config)
@@ -152,7 +147,7 @@ def test_apply_prompt_template_threads_explicit_app_config_to_subagents_without_
)
}
),
- skills=SimpleNamespace(container_path="/mnt/skills"),
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
skill_evolution=SimpleNamespace(enabled=False),
tool_search=SimpleNamespace(enabled=False),
memory=SimpleNamespace(enabled=False, injection_enabled=True, max_injection_tokens=2000),
@@ -284,7 +279,7 @@ def test_explicit_config_enabled_skills_are_cached_by_config_identity(monkeypatc
cast(
object,
SimpleNamespace(
- skills=SimpleNamespace(container_path="/mnt/skills"),
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
skill_evolution=SimpleNamespace(enabled=False),
),
),
@@ -304,6 +299,7 @@ def test_explicit_config_enabled_skills_are_cached_by_config_identity(monkeypatc
return SimpleNamespace(load_skills=load_skills)
monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", fake_get_or_new_skill_storage)
+ monkeypatch.setattr(prompt_module, "get_or_new_user_skill_storage", lambda user_id, **kwargs: SimpleNamespace(load_skills=lambda *, enabled_only: [make_skill("cached-skill")] if kwargs.get("app_config") is config else []))
_set_skills_cache_state()
try:
diff --git a/backend/tests/test_lead_agent_skills.py b/backend/tests/test_lead_agent_skills.py
index 7611d0c73..bb91a1de5 100644
--- a/backend/tests/test_lead_agent_skills.py
+++ b/backend/tests/test_lead_agent_skills.py
@@ -25,9 +25,26 @@ def _make_skill(name: str, allowed_tools: list[str] | None = None) -> Skill:
)
+def _mock_skill_storages(monkeypatch, skills):
+ """Patch storage factories and config so get_skills_prompt_section works without config.yaml."""
+ from types import SimpleNamespace
+
+ mock_storage = SimpleNamespace(load_skills=lambda *, enabled_only: skills)
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_skill_storage", lambda **kwargs: mock_storage)
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_user_skill_storage", lambda user_id, **kwargs: mock_storage)
+ monkeypatch.setattr(
+ "deerflow.config.get_app_config",
+ lambda: SimpleNamespace(
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
+ skill_evolution=SimpleNamespace(enabled=False),
+ ),
+ )
+
+
def test_get_skills_prompt_section_returns_empty_when_no_skills_match(monkeypatch):
skills = [_make_skill("skill1"), _make_skill("skill2")]
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills)
+ _mock_skill_storages(monkeypatch, skills)
result = get_skills_prompt_section(available_skills={"non_existent_skill"})
assert result == ""
@@ -36,6 +53,7 @@ def test_get_skills_prompt_section_returns_empty_when_no_skills_match(monkeypatc
def test_get_skills_prompt_section_returns_empty_when_available_skills_empty(monkeypatch):
skills = [_make_skill("skill1"), _make_skill("skill2")]
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills)
+ _mock_skill_storages(monkeypatch, skills)
result = get_skills_prompt_section(available_skills=set())
assert result == ""
@@ -44,6 +62,7 @@ def test_get_skills_prompt_section_returns_empty_when_available_skills_empty(mon
def test_get_skills_prompt_section_returns_skills(monkeypatch):
skills = [_make_skill("skill1"), _make_skill("skill2")]
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills)
+ _mock_skill_storages(monkeypatch, skills)
result = get_skills_prompt_section(available_skills={"skill1"})
assert "skill1" in result
@@ -54,6 +73,7 @@ def test_get_skills_prompt_section_returns_skills(monkeypatch):
def test_get_skills_prompt_section_returns_all_when_available_skills_is_none(monkeypatch):
skills = [_make_skill("skill1"), _make_skill("skill2")]
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills)
+ _mock_skill_storages(monkeypatch, skills)
result = get_skills_prompt_section(available_skills=None)
assert "skill1" in result
@@ -63,6 +83,7 @@ def test_get_skills_prompt_section_returns_all_when_available_skills_is_none(mon
def test_get_skills_prompt_section_includes_slash_activation_guidance(monkeypatch):
skills = [_make_skill("data-analysis")]
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills)
+ _mock_skill_storages(monkeypatch, skills)
result = get_skills_prompt_section(available_skills={"data-analysis"})
@@ -74,10 +95,12 @@ def test_get_skills_prompt_section_includes_slash_activation_guidance(monkeypatc
def test_get_skills_prompt_section_includes_self_evolution_rules(monkeypatch):
skills = [_make_skill("skill1")]
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills)
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_skill_storage", lambda **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: skills))
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_user_skill_storage", lambda user_id, **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: skills))
monkeypatch.setattr(
"deerflow.config.get_app_config",
lambda: SimpleNamespace(
- skills=SimpleNamespace(container_path="/mnt/skills"),
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
skill_evolution=SimpleNamespace(enabled=True),
),
)
@@ -88,10 +111,12 @@ def test_get_skills_prompt_section_includes_self_evolution_rules(monkeypatch):
def test_get_skills_prompt_section_includes_self_evolution_rules_without_skills(monkeypatch):
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: [])
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_skill_storage", lambda **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: []))
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_user_skill_storage", lambda user_id, **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: []))
monkeypatch.setattr(
"deerflow.config.get_app_config",
lambda: SimpleNamespace(
- skills=SimpleNamespace(container_path="/mnt/skills"),
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
skill_evolution=SimpleNamespace(enabled=True),
),
)
@@ -103,8 +128,10 @@ def test_get_skills_prompt_section_includes_self_evolution_rules_without_skills(
def test_get_skills_prompt_section_cache_respects_skill_evolution_toggle(monkeypatch):
skills = [_make_skill("skill1")]
monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills)
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_skill_storage", lambda **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: skills))
+ monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_user_skill_storage", lambda user_id, **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: skills))
config = SimpleNamespace(
- skills=SimpleNamespace(container_path="/mnt/skills"),
+ skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
skill_evolution=SimpleNamespace(enabled=True),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
@@ -119,7 +146,7 @@ def test_get_skills_prompt_section_cache_respects_skill_evolution_toggle(monkeyp
def test_get_skills_prompt_section_uses_explicit_config_for_enabled_skills(monkeypatch):
explicit_config = SimpleNamespace(
- skills=SimpleNamespace(container_path="/mnt/alt-skills"),
+ skills=SimpleNamespace(container_path="/mnt/alt-skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/alt-skills")),
skill_evolution=SimpleNamespace(enabled=False),
)
@@ -132,6 +159,10 @@ def test_get_skills_prompt_section_uses_explicit_config_for_enabled_skills(monke
"deerflow.agents.lead_agent.prompt.get_or_new_skill_storage",
lambda app_config=None, **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: [_make_skill("explicit-skill")] if app_config is explicit_config else []),
)
+ monkeypatch.setattr(
+ "deerflow.agents.lead_agent.prompt.get_or_new_user_skill_storage",
+ lambda user_id, app_config=None, **kwargs: __import__("types").SimpleNamespace(load_skills=lambda *, enabled_only: [_make_skill("explicit-skill")] if app_config is explicit_config else []),
+ )
result = get_skills_prompt_section(app_config=explicit_config)
@@ -149,7 +180,7 @@ def test_make_lead_agent_empty_skills_passed_correctly(monkeypatch):
monkeypatch.setattr(lead_agent_module, "_resolve_model_name", lambda x=None, **kwargs: "default-model")
monkeypatch.setattr(lead_agent_module, "create_chat_model", lambda **kwargs: "model")
monkeypatch.setattr("deerflow.tools.get_available_tools", lambda **kwargs: [])
- monkeypatch.setattr(lead_agent_module, "_load_enabled_skills_for_tool_policy", lambda available_skills, *, app_config: [])
+ monkeypatch.setattr(lead_agent_module, "_load_enabled_skills_for_tool_policy", lambda available_skills, *, app_config, user_id=None: [])
monkeypatch.setattr(lead_agent_module, "build_middlewares", lambda *args, **kwargs: [])
monkeypatch.setattr(lead_agent_module, "create_agent", lambda **kwargs: kwargs)
@@ -195,7 +226,7 @@ def test_make_lead_agent_filters_tools_from_available_skills(monkeypatch):
monkeypatch.setattr(lead_agent_module, "apply_prompt_template", lambda **kwargs: "mock_prompt")
monkeypatch.setattr(lead_agent_module, "create_agent", lambda **kwargs: kwargs)
monkeypatch.setattr(lead_agent_module, "load_agent_config", lambda x: AgentConfig(name="test", skills=["restricted", "legacy"]))
- monkeypatch.setattr(lead_agent_module, "_load_enabled_skills_for_tool_policy", lambda available_skills, *, app_config: [_make_skill("restricted", ["web_search"]), _make_skill("legacy", None)])
+ monkeypatch.setattr(lead_agent_module, "_load_enabled_skills_for_tool_policy", lambda available_skills, *, app_config, user_id=None: [_make_skill("restricted", ["read_file", "web_search"]), _make_skill("legacy", None)])
monkeypatch.setattr("deerflow.tools.get_available_tools", lambda **kwargs: [NamedTool("bash"), NamedTool("read_file"), NamedTool("web_search")])
mock_app_config = MagicMock()
@@ -229,7 +260,7 @@ def test_make_lead_agent_all_legacy_skills_preserve_all_tools(monkeypatch):
monkeypatch.setattr(lead_agent_module, "apply_prompt_template", lambda **kwargs: "mock_prompt")
monkeypatch.setattr(lead_agent_module, "create_agent", lambda **kwargs: kwargs)
monkeypatch.setattr(lead_agent_module, "load_agent_config", lambda x: AgentConfig(name="test", skills=None))
- monkeypatch.setattr(lead_agent_module, "_load_enabled_skills_for_tool_policy", lambda available_skills, *, app_config: [_make_skill("legacy", None)])
+ monkeypatch.setattr(lead_agent_module, "_load_enabled_skills_for_tool_policy", lambda available_skills, *, app_config, user_id=None: [_make_skill("legacy", None)])
monkeypatch.setattr("deerflow.tools.get_available_tools", lambda **kwargs: [NamedTool("bash"), NamedTool("read_file")])
mock_app_config = MagicMock()
@@ -262,6 +293,7 @@ def test_make_lead_agent_enforces_allowed_tools_when_skill_cache_is_cold(monkeyp
with prompt_module._enabled_skills_lock:
prompt_module._enabled_skills_cache = None
monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda app_config=None, **kwargs: mock_storage)
+ monkeypatch.setattr(prompt_module, "get_or_new_user_skill_storage", lambda user_id, app_config=None, **kwargs: mock_storage)
monkeypatch.setattr(lead_agent_module, "get_app_config", lambda: mock_app_config)
agent_kwargs = lead_agent_module.make_lead_agent({"configurable": {"agent_name": "test"}})
@@ -290,6 +322,7 @@ def test_make_lead_agent_fails_closed_when_skill_policy_load_fails(monkeypatch):
raise RuntimeError("skill storage unavailable")
monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", fail_storage)
+ monkeypatch.setattr(prompt_module, "get_or_new_user_skill_storage", fail_storage)
monkeypatch.setattr(lead_agent_module, "get_app_config", lambda: mock_app_config)
with pytest.raises(RuntimeError, match="skill storage unavailable"):
diff --git a/backend/tests/test_local_sandbox_provider_mounts.py b/backend/tests/test_local_sandbox_provider_mounts.py
index 8f87d03f8..1628ad9f5 100644
--- a/backend/tests/test_local_sandbox_provider_mounts.py
+++ b/backend/tests/test_local_sandbox_provider_mounts.py
@@ -547,6 +547,8 @@ class TestLocalSandboxProviderMounts:
def test_setup_path_mappings_uses_configured_skills_container_path_as_reserved_prefix(self, tmp_path):
skills_dir = tmp_path / "skills"
skills_dir.mkdir()
+ public_dir = skills_dir / "public"
+ public_dir.mkdir()
custom_dir = tmp_path / "custom"
custom_dir.mkdir()
@@ -566,11 +568,17 @@ class TestLocalSandboxProviderMounts:
with patch("deerflow.config.get_app_config", return_value=config):
provider = LocalSandboxProvider()
- assert [m.container_path for m in provider._path_mappings] == ["/custom-skills"]
+ # Public skills are the only static skills mount; custom skills are
+ # per-user and built dynamically in _build_thread_path_mappings.
+ # Custom volume mount /custom-skills/nested is also included (not
+ # a reserved prefix like /custom-skills/custom).
+ assert [m.container_path for m in provider._path_mappings] == ["/custom-skills/public", "/custom-skills/nested"]
def test_setup_path_mappings_skips_relative_host_path(self, tmp_path):
skills_dir = tmp_path / "skills"
skills_dir.mkdir()
+ public_dir = skills_dir / "public"
+ public_dir.mkdir()
from deerflow.config.sandbox_config import SandboxConfig, VolumeMountConfig
@@ -588,11 +596,14 @@ class TestLocalSandboxProviderMounts:
with patch("deerflow.config.get_app_config", return_value=config):
provider = LocalSandboxProvider()
- assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills"]
+ # Public skills mount is static; custom skills are per-thread.
+ assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills/public"]
def test_setup_path_mappings_skips_non_absolute_container_path(self, tmp_path):
skills_dir = tmp_path / "skills"
skills_dir.mkdir()
+ public_dir = skills_dir / "public"
+ public_dir.mkdir()
custom_dir = tmp_path / "custom"
custom_dir.mkdir()
@@ -612,7 +623,7 @@ class TestLocalSandboxProviderMounts:
with patch("deerflow.config.get_app_config", return_value=config):
provider = LocalSandboxProvider()
- assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills"]
+ assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills/public"]
def test_setup_path_mappings_logs_actionable_error_for_missing_host_path(self, tmp_path, caplog):
"""Regression for #3244.
@@ -626,6 +637,8 @@ class TestLocalSandboxProviderMounts:
"""
skills_dir = tmp_path / "skills"
skills_dir.mkdir()
+ public_dir = skills_dir / "public"
+ public_dir.mkdir()
missing_host_path = tmp_path / "does-not-exist"
from deerflow.config.sandbox_config import SandboxConfig, VolumeMountConfig
@@ -646,7 +659,8 @@ class TestLocalSandboxProviderMounts:
provider = LocalSandboxProvider()
# Silent-skip behaviour is preserved (no breaking change for existing deployments).
- assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills"]
+ # Only public skills mount is static; custom skills are per-thread.
+ assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills/public"]
# The failure must be observable at ERROR level and reference the offending paths.
error_records = [r for r in caplog.records if r.levelname == "ERROR"]
@@ -757,6 +771,8 @@ class TestLocalSandboxProviderMounts:
def test_setup_path_mappings_normalizes_container_path_trailing_slash(self, tmp_path):
skills_dir = tmp_path / "skills"
skills_dir.mkdir()
+ public_dir = skills_dir / "public"
+ public_dir.mkdir()
custom_dir = tmp_path / "custom"
custom_dir.mkdir()
@@ -776,7 +792,7 @@ class TestLocalSandboxProviderMounts:
with patch("deerflow.config.get_app_config", return_value=config):
provider = LocalSandboxProvider()
- assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills", "/mnt/data"]
+ assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills/public", "/mnt/data"]
class TestLocalSandboxProviderResetClearsSingleton:
diff --git a/backend/tests/test_local_skill_storage_write.py b/backend/tests/test_local_skill_storage_write.py
index 3abd4a63c..e2b40781c 100644
--- a/backend/tests/test_local_skill_storage_write.py
+++ b/backend/tests/test_local_skill_storage_write.py
@@ -4,10 +4,20 @@ from __future__ import annotations
import os
import stat
+from unittest.mock import patch
import pytest
-from deerflow.skills.storage import get_or_new_skill_storage
+from deerflow.config.paths import Paths
+from deerflow.skills.storage import get_or_new_skill_storage, reset_skill_storage
+from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
+
+
+@pytest.fixture(autouse=True)
+def _reset_storages():
+ reset_skill_storage()
+ yield
+ reset_skill_storage()
@pytest.fixture()
@@ -15,6 +25,15 @@ def storage(tmp_path):
return get_or_new_skill_storage(skills_path=str(tmp_path))
+@pytest.fixture()
+def user_storage(tmp_path):
+ """UserScopedSkillStorage for user 'test-user'."""
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=tmp_path)):
+ with patch("deerflow.config.paths._paths", None):
+ s = UserScopedSkillStorage("test-user", host_path=str(tmp_path))
+ return s
+
+
@pytest.fixture()
def skill_dir(tmp_path, storage):
"""Pre-create the skill directory so symlink tests can plant files inside."""
@@ -23,6 +42,14 @@ def skill_dir(tmp_path, storage):
return d
+@pytest.fixture()
+def user_skill_dir(tmp_path, user_storage):
+ """Pre-create the user-scoped skill directory."""
+ d = tmp_path / "users" / "test-user" / "skills" / "custom" / "demo-skill"
+ d.mkdir(parents=True, exist_ok=True)
+ return d
+
+
# ---------------------------------------------------------------------------
# Happy path
# ---------------------------------------------------------------------------
@@ -175,3 +202,42 @@ def test_rejects_invalid_skill_name_in_path_helpers(storage, name, method_name):
method = getattr(storage, method_name)
with pytest.raises(ValueError, match="hyphen-case"):
method(name)
+
+
+# ---------------------------------------------------------------------------
+# UserScopedSkillStorage write tests
+# ---------------------------------------------------------------------------
+
+
+def test_user_scoped_write_creates_file_in_user_dir(tmp_path, user_storage):
+ user_storage.write_custom_skill("demo-skill", "SKILL.md", "# hello")
+ user_file = tmp_path / "users" / "test-user" / "skills" / "custom" / "demo-skill" / "SKILL.md"
+ assert user_file.read_text() == "# hello"
+ # Does not create in global custom
+ assert not (tmp_path / "custom" / "demo-skill" / "SKILL.md").exists()
+
+
+def test_user_scoped_write_creates_subdirectory(tmp_path, user_storage):
+ user_storage.write_custom_skill("demo-skill", "references/ref.md", "# ref")
+ assert (tmp_path / "users" / "test-user" / "skills" / "custom" / "demo-skill" / "references" / "ref.md").exists()
+
+
+def test_user_scoped_write_is_atomic_overwrite(tmp_path, user_storage):
+ user_storage.write_custom_skill("demo-skill", "SKILL.md", "first")
+ user_storage.write_custom_skill("demo-skill", "SKILL.md", "second")
+ assert (tmp_path / "users" / "test-user" / "skills" / "custom" / "demo-skill" / "SKILL.md").read_text() == "second"
+
+
+def test_user_scoped_rejects_empty_string(user_storage):
+ with pytest.raises(ValueError, match="empty"):
+ user_storage.write_custom_skill("demo-skill", "", "x")
+
+
+def test_user_scoped_rejects_dotdot_escape(user_storage):
+ with pytest.raises(ValueError, match="skill directory"):
+ user_storage.write_custom_skill("demo-skill", "../../escaped.txt", "x")
+
+
+def test_user_scoped_rejects_invalid_skill_name(user_storage):
+ with pytest.raises(ValueError, match="hyphen-case"):
+ user_storage.get_custom_skill_dir("../../escaped")
diff --git a/backend/tests/test_mcp_file_migration.py b/backend/tests/test_mcp_file_migration.py
index ccdcadaaf..615008b63 100644
--- a/backend/tests/test_mcp_file_migration.py
+++ b/backend/tests/test_mcp_file_migration.py
@@ -327,11 +327,19 @@ class TestRewriteLocalPathsInText:
class TestWorkspaceSnapshots:
def test_changed_workspace_files_detects_created_and_modified_files(self, paths: Paths):
+ import time
+
workspace = paths.sandbox_work_dir("t1", user_id="u1")
existing = _workspace_file(paths, "existing.txt", content=b"old")
before = mcp_tools._snapshot_workspace_files(workspace)
- existing.write_bytes(b"new")
+ # Ensure the mtime advances so the change is detectable. Without the
+ # sleep, write_bytes(b"new") may land in the same nanosecond as the
+ # snapshot, and since b"old" and b"new" have the same length, the
+ # (mtime_ns, size) signature stays identical → _changed_workspace_files
+ # misses the modification.
+ time.sleep(0.05)
+ existing.write_bytes(b"new_content") # different length guarantees size change too
created = _workspace_file(paths, "created.txt", content=b"created")
changed = set(mcp_tools._changed_workspace_files(workspace, before))
diff --git a/backend/tests/test_migration_user_isolation.py b/backend/tests/test_migration_user_isolation.py
index da979e8f7..0f300a031 100644
--- a/backend/tests/test_migration_user_isolation.py
+++ b/backend/tests/test_migration_user_isolation.py
@@ -190,3 +190,92 @@ class TestMigrateAgents:
report = migrate_agents(paths, user_id="default")
assert report == []
+
+
+class TestMigrateSkills:
+ @staticmethod
+ def _seed_legacy_skill(base_dir: Path, name: str, *, content: str = "skill doc") -> Path:
+ skill_dir = base_dir / "skills" / "custom" / name
+ skill_dir.mkdir(parents=True, exist_ok=True)
+ (skill_dir / "SKILL.md").write_text(content, encoding="utf-8")
+ return skill_dir
+
+ def test_moves_legacy_into_user_layout(self, base_dir: Path, paths: Paths):
+ self._seed_legacy_skill(base_dir, "my-skill", content="legacy skill")
+ (base_dir / "skills" / "public" / "bootstrap").mkdir(parents=True)
+
+ from scripts.migrate_user_isolation import migrate_skills
+
+ report = migrate_skills(paths, user_id="default")
+
+ assert len(report) == 1
+ assert report[0]["skill"] == "my-skill"
+ assert "moved -> " in report[0]["action"]
+
+ dest = paths.user_custom_skills_dir("default") / "my-skill" / "SKILL.md"
+ assert dest.exists()
+ assert dest.read_text() == "legacy skill"
+ # Legacy custom dir cleaned up
+ assert not (base_dir / "skills" / "custom").exists()
+ # But skills/ parent survives (public/ still in use)
+ assert (base_dir / "skills" / "public").exists()
+
+ def test_dry_run_does_not_move(self, base_dir: Path, paths: Paths):
+ legacy_dir = self._seed_legacy_skill(base_dir, "my-skill")
+
+ from scripts.migrate_user_isolation import migrate_skills
+
+ report = migrate_skills(paths, user_id="default", dry_run=True)
+
+ assert len(report) == 1
+ assert legacy_dir.exists(), "dry-run must not touch the filesystem"
+ assert not (paths.user_custom_skills_dir("default") / "my-skill").exists()
+
+ def test_existing_destination_is_conflict(self, base_dir: Path, paths: Paths):
+ self._seed_legacy_skill(base_dir, "my-skill", content="legacy")
+ dest = paths.user_custom_skills_dir("default") / "my-skill"
+ dest.mkdir(parents=True)
+ (dest / "SKILL.md").write_text("preexisting", encoding="utf-8")
+
+ from scripts.migrate_user_isolation import migrate_skills
+
+ report = migrate_skills(paths, user_id="default")
+
+ assert report[0]["action"].startswith("conflict -> ")
+ assert (dest / "SKILL.md").read_text() == "preexisting"
+ conflicts_dir = paths.base_dir / "migration-conflicts" / "skills" / "my-skill"
+ assert (conflicts_dir / "SKILL.md").read_text() == "legacy"
+
+ def test_no_legacy_dir_is_noop(self, base_dir: Path, paths: Paths):
+ from scripts.migrate_user_isolation import migrate_skills
+
+ report = migrate_skills(paths, user_id="default")
+ assert report == []
+
+ def test_migrates_history_dir(self, base_dir: Path, paths: Paths):
+ history_dir = base_dir / "skills" / "custom" / ".history"
+ history_dir.mkdir(parents=True)
+ (history_dir / "log.json").write_text("[]", encoding="utf-8")
+
+ from scripts.migrate_user_isolation import migrate_skills
+
+ migrate_skills(paths, user_id="default")
+
+ dest_history = paths.user_custom_skills_dir("default") / ".history" / "log.json"
+ assert dest_history.exists()
+ assert not history_dir.exists()
+
+ def test_skills_parent_dir_not_deleted_even_if_custom_empty(self, base_dir: Path, paths: Paths):
+ """skills/ parent must NOT be deleted — public/ may still be in use."""
+ # Create only custom dir (empty), public dir with content
+ (base_dir / "skills" / "custom").mkdir(parents=True)
+ (base_dir / "skills" / "public" / "bootstrap").mkdir(parents=True)
+
+ from scripts.migrate_user_isolation import migrate_skills
+
+ migrate_skills(paths, user_id="default")
+
+ # custom/ cleaned up (was empty), but skills/ survives
+ assert not (base_dir / "skills" / "custom").exists()
+ assert (base_dir / "skills").exists()
+ assert (base_dir / "skills" / "public").exists()
diff --git a/backend/tests/test_sandbox_search_tools.py b/backend/tests/test_sandbox_search_tools.py
index 30e9d384e..4e7b0fb08 100644
--- a/backend/tests/test_sandbox_search_tools.py
+++ b/backend/tests/test_sandbox_search_tools.py
@@ -2,7 +2,7 @@ from types import SimpleNamespace
from unittest.mock import patch
from deerflow.community.aio_sandbox.aio_sandbox import AioSandbox
-from deerflow.sandbox.local.local_sandbox import LocalSandbox
+from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping
from deerflow.sandbox.search import GrepMatch, find_glob_matches, find_grep_matches
from deerflow.sandbox.tools import glob_tool, grep_tool, ls_tool
@@ -57,18 +57,20 @@ def test_glob_tool_supports_skills_virtual_paths(tmp_path, monkeypatch) -> None:
(skills_dir / "public" / "demo").mkdir(parents=True)
(skills_dir / "public" / "demo" / "SKILL.md").write_text("# Demo\n", encoding="utf-8")
- monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: LocalSandbox(id="local"))
+ sandbox = LocalSandbox(
+ id="local",
+ path_mappings=[
+ PathMapping(container_path="/mnt/skills", local_path=str(skills_dir), read_only=True),
+ ],
+ )
+ monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox)
- with (
- patch("deerflow.sandbox.tools._get_skills_container_path", return_value="/mnt/skills"),
- patch("deerflow.sandbox.tools._get_skills_host_path", return_value=str(skills_dir)),
- ):
- result = glob_tool.func(
- runtime=runtime,
- description="find skills",
- pattern="**/SKILL.md",
- path="/mnt/skills",
- )
+ result = glob_tool.func(
+ runtime=runtime,
+ description="find skills",
+ pattern="**/SKILL.md",
+ path="/mnt/skills",
+ )
assert "/mnt/skills/public/demo/SKILL.md" in result
assert str(skills_dir) not in result
@@ -427,17 +429,19 @@ def test_ls_tool_masks_skills_host_paths(tmp_path, monkeypatch) -> None:
(skills_dir / "public").mkdir(parents=True)
(skills_dir / "public" / "SKILL.md").write_text("# Skill\n", encoding="utf-8")
- monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: LocalSandbox(id="local"))
+ sandbox = LocalSandbox(
+ id="local",
+ path_mappings=[
+ PathMapping(container_path="/mnt/skills", local_path=str(skills_dir), read_only=True),
+ ],
+ )
+ monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox)
- with (
- patch("deerflow.sandbox.tools._get_skills_container_path", return_value="/mnt/skills"),
- patch("deerflow.sandbox.tools._get_skills_host_path", return_value=str(skills_dir)),
- ):
- result = ls_tool.func(
- runtime=runtime,
- description="list skills",
- path="/mnt/skills",
- )
+ result = ls_tool.func(
+ runtime=runtime,
+ description="list skills",
+ path="/mnt/skills",
+ )
# Virtual paths must be present
assert "/mnt/skills" in result
@@ -461,6 +465,59 @@ def test_ls_tool_returns_empty_for_empty_directory(tmp_path, monkeypatch) -> Non
assert result == "(empty)"
+def test_ls_tool_skills_path_uses_sandbox_mapping_user_id_not_contextvar(tmp_path, monkeypatch) -> None:
+ """ls_tool must resolve /mnt/skills/custom via the sandbox PathMapping
+ (which uses the user_id from acquire time), not via _resolve_skills_path
+ (which uses get_effective_user_id() from contextvar).
+
+ Regression: when the contextvar user_id differs from the sandbox mapping's
+ user_id (e.g., contextvar unset → "default", but sandbox uses authenticated
+ "user-abc"), _resolve_skills_path would resolve to the wrong directory,
+ making /mnt/skills/custom appear empty. The fix delegates resolution to the
+ sandbox's PathMapping which always uses the acquire-time user_id.
+ """
+ from deerflow.runtime.user_context import reset_current_user, set_current_user
+
+ # Create two user-specific custom skill directories:
+ # - user-abc: has a skill "my-skill"
+ # - default: empty (the fallback when contextvar is unset)
+ base_dir = tmp_path / ".deer-flow"
+ user_abc_custom = base_dir / "users" / "user-abc" / "skills" / "custom"
+ user_abc_custom.mkdir(parents=True)
+ (user_abc_custom / "my-skill").mkdir()
+ (user_abc_custom / "my-skill" / "SKILL.md").write_text("# My Skill\n", encoding="utf-8")
+
+ default_custom = base_dir / "users" / "default" / "skills" / "custom"
+ default_custom.mkdir(parents=True) # exists but empty
+
+ # Create a sandbox with PathMappings that use user-abc's directory
+ # (simulating a sandbox acquired for user-abc)
+ sandbox = LocalSandbox(
+ id="local:user-abc:thread-1",
+ path_mappings=[
+ PathMapping(container_path="/mnt/skills/custom", local_path=str(user_abc_custom), read_only=True),
+ ],
+ )
+ monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox)
+
+ # Leave contextvar unset → get_effective_user_id() returns "default"
+ # Before the fix, _resolve_skills_path would resolve to default_custom (empty)
+ # After the fix, the sandbox PathMapping resolves to user-abc_custom (has my-skill)
+ token = set_current_user(SimpleNamespace(id="default")) # contextvar says "default"
+ try:
+ result = ls_tool.func(
+ runtime=_make_runtime(tmp_path),
+ description="list custom skills",
+ path="/mnt/skills/custom",
+ )
+
+ # Must show user-abc's skill (sandbox mapping), NOT default's empty dir (contextvar)
+ assert "my-skill" in result
+ assert str(user_abc_custom) not in result # host paths must not leak
+ finally:
+ reset_current_user(token)
+
+
def test_ls_tool_filters_upload_staging_files(tmp_path, monkeypatch) -> None:
runtime = _make_runtime(tmp_path)
uploads = tmp_path / "uploads"
diff --git a/backend/tests/test_sandbox_tools_security.py b/backend/tests/test_sandbox_tools_security.py
index 4b4687b41..43af56b01 100644
--- a/backend/tests/test_sandbox_tools_security.py
+++ b/backend/tests/test_sandbox_tools_security.py
@@ -306,29 +306,39 @@ def test_resolve_and_validate_user_data_path_blocks_traversal(tmp_path: Path) ->
# ---------- replace_virtual_paths_in_command ----------
-def test_replace_virtual_paths_in_command_replaces_skills_paths() -> None:
- """Skills virtual paths in commands should be resolved to host paths."""
+def test_replace_virtual_paths_in_command_does_not_replace_skills_paths() -> None:
+ """Skills virtual paths in commands should NOT be resolved by replace_virtual_paths_in_command.
+
+ Skills and ACP workspace paths are resolved by the sandbox's
+ PathMapping at execution time, not by pre-resolving in
+ replace_virtual_paths_in_command, because the sandbox's user_id
+ (from acquire time) may differ from the contextvar user_id used by
+ _resolve_skills_path / _resolve_acp_workspace_path.
+ """
with (
patch("deerflow.sandbox.tools._get_skills_container_path", return_value="/mnt/skills"),
patch("deerflow.sandbox.tools._get_skills_host_path", return_value="/home/user/deer-flow/skills"),
):
cmd = "cat /mnt/skills/public/bootstrap/SKILL.md"
result = replace_virtual_paths_in_command(cmd, _THREAD_DATA)
- assert "/mnt/skills" not in result
- assert "/home/user/deer-flow/skills/public/bootstrap/SKILL.md" in result
+ # Skills paths should remain as virtual paths (not resolved)
+ assert "/mnt/skills/public/bootstrap/SKILL.md" in result
+ assert "/home/user/deer-flow/skills" not in result
-def test_replace_virtual_paths_in_command_replaces_both() -> None:
- """Both user-data and skills paths should be replaced in the same command."""
+def test_replace_virtual_paths_in_command_replaces_user_data_only() -> None:
+ """Only user-data paths should be replaced; skills and ACP paths stay virtual."""
with (
patch("deerflow.sandbox.tools._get_skills_container_path", return_value="/mnt/skills"),
patch("deerflow.sandbox.tools._get_skills_host_path", return_value="/home/user/skills"),
):
cmd = "cat /mnt/skills/public/SKILL.md > /mnt/user-data/workspace/out.txt"
result = replace_virtual_paths_in_command(cmd, _THREAD_DATA)
- assert "/mnt/skills" not in result
+ # Skills paths should remain virtual
+ assert "/mnt/skills/public/SKILL.md" in result
+ assert "/home/user/skills" not in result
+ # User-data paths should still be resolved
assert "/mnt/user-data" not in result
- assert "/home/user/skills/public/SKILL.md" in result
assert "/tmp/deer-flow/threads/t1/user-data/workspace/out.txt" in result
@@ -769,14 +779,22 @@ def test_resolve_acp_workspace_path_blocks_traversal(tmp_path: Path) -> None:
_resolve_acp_workspace_path("/mnt/acp-workspace/../../etc/passwd")
-def test_replace_virtual_paths_in_command_replaces_acp_workspace() -> None:
- """ACP workspace virtual paths in commands should be resolved to host paths."""
+def test_replace_virtual_paths_in_command_does_not_replace_acp_workspace() -> None:
+ """ACP workspace virtual paths should NOT be resolved by replace_virtual_paths_in_command.
+
+ Like skills paths, ACP workspace paths are resolved by the sandbox's
+ PathMapping at execution time, not pre-resolved, to ensure user_id
+ consistency with the sandbox mapping.
+ """
acp_host = "/home/user/.deer-flow/acp-workspace"
with patch("deerflow.sandbox.tools._get_acp_workspace_host_path", return_value=acp_host):
cmd = "cp /mnt/acp-workspace/hello.py /mnt/user-data/outputs/hello.py"
result = replace_virtual_paths_in_command(cmd, _THREAD_DATA)
- assert "/mnt/acp-workspace" not in result
- assert f"{acp_host}/hello.py" in result
+ # ACP workspace path should remain as virtual path (not resolved)
+ assert "/mnt/acp-workspace/hello.py" in result
+ assert acp_host not in result
+ # User-data paths should still be resolved
+ assert "/mnt/user-data" not in result
assert "/tmp/deer-flow/threads/t1/user-data/outputs/hello.py" in result
diff --git a/backend/tests/test_sandbox_windows_path_normalization.py b/backend/tests/test_sandbox_windows_path_normalization.py
index 211537cd6..2a93636e8 100644
--- a/backend/tests/test_sandbox_windows_path_normalization.py
+++ b/backend/tests/test_sandbox_windows_path_normalization.py
@@ -42,14 +42,6 @@ class TestReplaceVirtualPathsWindows:
result = replace_virtual_paths_in_command(cmd, _WIN_THREAD_DATA)
assert "\\" not in result, f"Backslash in: {result}"
- @patch("deerflow.sandbox.tools._resolve_acp_workspace_path", return_value=r"C:\Users\admin\deer-flow\acp-workspace\data.json")
- @patch("deerflow.sandbox.tools._get_acp_workspace_host_path", return_value=r"C:\Users\admin\deer-flow\acp-workspace")
- def test_acp_workspace_no_backslash(self, _mock_acp_host, _mock_resolve_acp) -> None:
- cmd = "cat /mnt/acp-workspace/data.json"
- result = replace_virtual_paths_in_command(cmd, _WIN_THREAD_DATA)
- assert "\\" not in result, f"Backslash in: {result}"
- assert "C:/Users/admin/deer-flow/acp-workspace/data.json" in result
-
class TestLocalSandboxResolvePathsInCommandWindows:
"""LocalSandbox._resolve_paths_in_command must normalize backslashes."""
@@ -77,3 +69,22 @@ class TestLocalSandboxResolvePathsInCommandWindows:
result = sandbox._resolve_paths_in_command(cmd)
assert "\\" not in result, f"Backslash in: {result}"
assert "C:/Users/admin/data/workspace/file.txt" in result
+
+ def test_acp_workspace_no_backslash(self) -> None:
+ """PR #3889 moved ACP workspace path resolution from
+ ``replace_virtual_paths_in_command`` to ``LocalSandbox._resolve_paths_in_command``
+ via ``PathMapping``. Verify the Windows backslash normalization still
+ applies to ACP workspace paths through the new routing — the same
+ guarantee ``test_acp_workspace_no_backslash`` (now removed) provided
+ for the legacy inline code path.
+ """
+ sandbox = LocalSandbox(
+ "test",
+ path_mappings=[
+ PathMapping(container_path="/mnt/acp-workspace", local_path=r"C:\Users\admin\deer-flow\acp-workspace", read_only=False),
+ ],
+ )
+ cmd = "cat /mnt/acp-workspace/data.json"
+ result = sandbox._resolve_paths_in_command(cmd)
+ assert "\\" not in result, f"Backslash in: {result}"
+ assert "C:/Users/admin/deer-flow/acp-workspace/data.json" in result
diff --git a/backend/tests/test_skill_manage_tool.py b/backend/tests/test_skill_manage_tool.py
index 3933cb208..a3395b0f5 100644
--- a/backend/tests/test_skill_manage_tool.py
+++ b/backend/tests/test_skill_manage_tool.py
@@ -1,4 +1,5 @@
import importlib
+from pathlib import Path
from types import SimpleNamespace
import anyio
@@ -17,23 +18,44 @@ async def _async_result(decision: str, reason: str):
return ScanResult(decision=decision, reason=reason)
-def test_skill_manage_create_and_patch(monkeypatch, tmp_path):
- skills_root = tmp_path / "skills"
- config = SimpleNamespace(
- skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
+def _make_config(skills_root: Path):
+ return SimpleNamespace(
+ skills=SimpleNamespace(
+ get_skills_path=lambda: skills_root,
+ container_path="/mnt/skills",
+ use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
+ ),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
+
+
+def _make_runtime(*, thread_id: str = "thread-1", user_id: str = "default"):
+ return SimpleNamespace(
+ context={"thread_id": thread_id, "user_id": user_id},
+ config={"configurable": {"thread_id": thread_id, "user_id": user_id}},
+ )
+
+
+def test_skill_manage_create_and_patch(monkeypatch, tmp_path):
+ skills_root = tmp_path / "skills"
+ config = _make_config(skills_root)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config)
+ # Patch get_paths so UserScopedSkillStorage resolves user dirs under tmp_path
+ from deerflow.config.paths import Paths
+
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
refresh_calls = []
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
- monkeypatch.setattr(skill_manage_module, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skill_manage_module, "refresh_user_skills_system_prompt_cache_async", _refresh)
monkeypatch.setattr(skill_manage_module, "scan_skill_content", lambda *args, **kwargs: _async_result("allow", "ok"))
- runtime = SimpleNamespace(context={"thread_id": "thread-1"}, config={"configurable": {"thread_id": "thread-1"}})
+ runtime = _make_runtime(user_id="default")
result = anyio.run(
skill_manage_module.skill_manage_tool.coroutine,
@@ -56,26 +78,29 @@ def test_skill_manage_create_and_patch(monkeypatch, tmp_path):
1,
)
assert "Patched custom skill" in patch_result
- assert "Patched skill" in (skills_root / "custom" / "demo-skill" / "SKILL.md").read_text(encoding="utf-8")
- assert refresh_calls == ["refresh", "refresh"]
+ # User-scoped: custom skills written under users/default/skills/custom/
+ user_custom = tmp_path / "users" / "default" / "skills" / "custom"
+ assert "Patched skill" in (user_custom / "demo-skill" / "SKILL.md").read_text(encoding="utf-8")
+ assert refresh_calls == [("refresh", "default"), ("refresh", "default")]
def test_skill_manage_patch_replaces_single_occurrence_by_default(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- config = SimpleNamespace(
- skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
- skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
- )
+ config = _make_config(skills_root)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config)
+ from deerflow.config.paths import Paths
- async def _refresh():
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ async def _refresh(user_id: str):
return None
- monkeypatch.setattr(skill_manage_module, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skill_manage_module, "refresh_user_skills_system_prompt_cache_async", _refresh)
monkeypatch.setattr(skill_manage_module, "scan_skill_content", lambda *args, **kwargs: _async_result("allow", "ok"))
- runtime = SimpleNamespace(context={"thread_id": "thread-1"}, config={"configurable": {"thread_id": "thread-1"}})
+ runtime = _make_runtime(user_id="default")
content = _skill_content("demo-skill", "Demo skill") + "\nRepeated: Demo skill\n"
anyio.run(skill_manage_module.skill_manage_tool.coroutine, runtime, "create", "demo-skill", content)
@@ -90,7 +115,8 @@ def test_skill_manage_patch_replaces_single_occurrence_by_default(monkeypatch, t
"Patched skill",
)
- skill_text = (skills_root / "custom" / "demo-skill" / "SKILL.md").read_text(encoding="utf-8")
+ user_custom = tmp_path / "users" / "default" / "skills" / "custom"
+ skill_text = (user_custom / "demo-skill" / "SKILL.md").read_text(encoding="utf-8")
assert "1 replacement(s) applied, 2 match(es) found" in patch_result
assert skill_text.count("Patched skill") == 1
assert skill_text.count("Demo skill") == 1
@@ -101,13 +127,14 @@ def test_skill_manage_rejects_public_skill_patch(monkeypatch, tmp_path):
public_dir = skills_root / "public" / "deep-research"
public_dir.mkdir(parents=True, exist_ok=True)
(public_dir / "SKILL.md").write_text(_skill_content("deep-research"), encoding="utf-8")
- config = SimpleNamespace(
- skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
- skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
- )
+ config = _make_config(skills_root)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+ from deerflow.config.paths import Paths
- runtime = SimpleNamespace(context={}, config={"configurable": {}})
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ runtime = _make_runtime(user_id="default")
with pytest.raises(ValueError, match="built-in skill"):
anyio.run(
@@ -124,20 +151,22 @@ def test_skill_manage_rejects_public_skill_patch(monkeypatch, tmp_path):
def test_skill_manage_sync_wrapper_supported(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- config = SimpleNamespace(
- skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
- skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
- )
+ config = _make_config(skills_root)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+ from deerflow.config.paths import Paths
+
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
refresh_calls = []
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
- monkeypatch.setattr(skill_manage_module, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skill_manage_module, "refresh_user_skills_system_prompt_cache_async", _refresh)
monkeypatch.setattr(skill_manage_module, "scan_skill_content", lambda *args, **kwargs: _async_result("allow", "ok"))
- runtime = SimpleNamespace(context={"thread_id": "thread-sync"}, config={"configurable": {"thread_id": "thread-sync"}})
+ runtime = _make_runtime(thread_id="thread-sync", user_id="default")
result = skill_manage_module.skill_manage_tool.func(
runtime=runtime,
action="create",
@@ -146,25 +175,26 @@ def test_skill_manage_sync_wrapper_supported(monkeypatch, tmp_path):
)
assert "Created custom skill" in result
- assert refresh_calls == ["refresh"]
+ assert refresh_calls == [("refresh", "default")]
def test_skill_manage_rejects_support_path_traversal(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- config = SimpleNamespace(
- skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
- skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
- )
+ config = _make_config(skills_root)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config)
+ from deerflow.config.paths import Paths
- async def _refresh():
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ async def _refresh(user_id: str):
return None
- monkeypatch.setattr(skill_manage_module, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skill_manage_module, "refresh_user_skills_system_prompt_cache_async", _refresh)
monkeypatch.setattr(skill_manage_module, "scan_skill_content", lambda *args, **kwargs: _async_result("allow", "ok"))
- runtime = SimpleNamespace(context={"thread_id": "thread-1"}, config={"configurable": {"thread_id": "thread-1"}})
+ runtime = _make_runtime(user_id="default")
anyio.run(skill_manage_module.skill_manage_tool.coroutine, runtime, "create", "demo-skill", _skill_content("demo-skill"))
with pytest.raises(ValueError, match="parent-directory traversal|selected support directory"):
@@ -176,3 +206,52 @@ def test_skill_manage_rejects_support_path_traversal(monkeypatch, tmp_path):
"malicious overwrite",
"references/../SKILL.md",
)
+
+
+def test_skill_manage_per_user_isolation(monkeypatch, tmp_path):
+ """Two different users must get separate custom skill directories."""
+ skills_root = tmp_path / "skills"
+ config = _make_config(skills_root)
+ monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+ monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config)
+ from deerflow.config.paths import Paths
+
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ async def _refresh(user_id: str):
+ return None
+
+ monkeypatch.setattr(skill_manage_module, "refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skill_manage_module, "scan_skill_content", lambda *args, **kwargs: _async_result("allow", "ok"))
+
+ # Alice creates a skill
+ runtime_alice = _make_runtime(user_id="alice")
+ result_a = anyio.run(
+ skill_manage_module.skill_manage_tool.coroutine,
+ runtime_alice,
+ "create",
+ "alice-skill",
+ _skill_content("alice-skill"),
+ )
+ assert "Created custom skill" in result_a
+
+ # Bob creates a different skill
+ runtime_bob = _make_runtime(user_id="bob")
+ result_b = anyio.run(
+ skill_manage_module.skill_manage_tool.coroutine,
+ runtime_bob,
+ "create",
+ "bob-skill",
+ _skill_content("bob-skill"),
+ )
+ assert "Created custom skill" in result_b
+
+ # Verify separate directories
+ alice_dir = tmp_path / "users" / "alice" / "skills" / "custom" / "alice-skill"
+ bob_dir = tmp_path / "users" / "bob" / "skills" / "custom" / "bob-skill"
+ assert alice_dir.exists()
+ assert bob_dir.exists()
+ # No cross-contamination
+ assert not (tmp_path / "users" / "alice" / "skills" / "custom" / "bob-skill").exists()
+ assert not (tmp_path / "users" / "bob" / "skills" / "custom" / "alice-skill").exists()
diff --git a/backend/tests/test_skill_storage_lifecycle.py b/backend/tests/test_skill_storage_lifecycle.py
index 9a4891a05..cab3966c8 100644
--- a/backend/tests/test_skill_storage_lifecycle.py
+++ b/backend/tests/test_skill_storage_lifecycle.py
@@ -1,10 +1,10 @@
"""Concurrency regression tests for the skill storage singleton lifecycle.
These guard the unsynchronized check-then-create in ``get_or_new_skill_storage``
-and the unlocked ``reset_skill_storage``: before the lock was added, concurrent
-cold-start callers could each construct a separate ``SkillStorage`` and overwrite
-the global, and a ``reset_skill_storage`` racing a get could hand a caller
-``None``.
+and ``get_or_new_user_skill_storage``, and the unlocked ``reset_skill_storage``:
+before the lock was added, concurrent cold-start callers could each construct a
+separate ``SkillStorage`` and overwrite the global, and a ``reset_skill_storage``
+racing a get could hand a caller ``None``.
This mirrors ``test_sandbox_provider_lifecycle.py`` — the sibling singleton that
``skills/storage/__init__.py`` documents itself as patterned after — adapted to
@@ -21,6 +21,7 @@ import time
from pathlib import Path
import deerflow.skills.storage as skill_storage
+from deerflow.config.paths import Paths
from deerflow.skills.storage import SkillStorage
@@ -170,3 +171,164 @@ def test_reset_racing_get_of_live_singleton_never_returns_none(monkeypatch):
assert all(isinstance(storage, SlowSkillStorage) for storage in results)
finally:
skill_storage.reset_skill_storage()
+
+
+# ---------------------------------------------------------------------------
+# Per-user skill storage lifecycle
+# ---------------------------------------------------------------------------
+
+
+class SlowUserSkillStorage(SkillStorage):
+ """Storage whose constructor is slow, to widen the check-then-create gap.
+
+ Signature mirrors ``UserScopedSkillStorage.__init__(user_id, **kwargs)``
+ so the module-level factory can call it as ``cls(user_id, **kwargs)``.
+ """
+
+ instances_created = 0
+ instances_lock = threading.Lock()
+
+ def __init__(self, user_id: str = "default", **kwargs) -> None:
+ super().__init__(container_path=kwargs.get("container_path", "/mnt/skills"))
+ self._user_id = user_id
+ time.sleep(0.05)
+ with self.instances_lock:
+ type(self).instances_created += 1
+
+ def get_skills_root_path(self) -> Path:
+ return Path("/tmp/skills")
+
+ def _iter_skill_files(self):
+ return []
+
+ def read_custom_skill(self, name: str) -> str:
+ return ""
+
+ def write_custom_skill(self, name: str, relative_path: str, content: str) -> None:
+ pass
+
+ async def ainstall_skill_from_archive(self, archive_path) -> dict:
+ return {}
+
+ def delete_custom_skill(self, name: str, *, history_meta: dict | None = None) -> None:
+ pass
+
+ def custom_skill_exists(self, name: str) -> bool:
+ return False
+
+ def public_skill_exists(self, name: str) -> bool:
+ return False
+
+ def append_history(self, name: str, record: dict) -> None:
+ pass
+
+ def read_history(self, name: str) -> list[dict]:
+ return []
+
+
+def _patch_user_storage_resolution(monkeypatch, cls=SlowUserSkillStorage) -> None:
+ monkeypatch.setattr("deerflow.config.get_app_config", lambda: _APP_CONFIG)
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=Path("/tmp")))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+ # get_or_new_user_skill_storage calls UserScopedSkillStorage(user_id, **kwargs)
+ # directly — not via resolve_class. Patch the class reference in the module.
+ monkeypatch.setattr("deerflow.skills.storage.UserScopedSkillStorage", cls)
+
+
+def test_get_or_new_user_skill_storage_constructs_one_per_user_under_concurrent_access(monkeypatch):
+ """Eight threads racing on a cold start for the same user_id must construct
+ exactly one instance per user. Different user_ids get different instances.
+ """
+ skill_storage.reset_skill_storage()
+ SlowUserSkillStorage.instances_created = 0
+ _patch_user_storage_resolution(monkeypatch)
+
+ n_threads = 8
+ storages: list[SkillStorage] = []
+ storages_lock = threading.Lock()
+ barrier = threading.Barrier(n_threads)
+
+ def get_storage() -> None:
+ barrier.wait()
+ storage = skill_storage.get_or_new_user_skill_storage("alice")
+ with storages_lock:
+ storages.append(storage)
+
+ threads = [threading.Thread(target=get_storage) for _ in range(n_threads)]
+ for thread in threads:
+ thread.start()
+ for thread in threads:
+ thread.join()
+
+ try:
+ assert len({id(storage) for storage in storages}) == 1
+ assert SlowUserSkillStorage.instances_created == 1
+ finally:
+ skill_storage.reset_skill_storage()
+
+
+def test_different_users_get_different_storages(monkeypatch):
+ """Concurrent calls for different user_ids must produce distinct instances."""
+ skill_storage.reset_skill_storage()
+ SlowUserSkillStorage.instances_created = 0
+ _patch_user_storage_resolution(monkeypatch)
+
+ s_alice = skill_storage.get_or_new_user_skill_storage("alice")
+ s_bob = skill_storage.get_or_new_user_skill_storage("bob")
+
+ try:
+ assert s_alice is not s_bob
+ assert s_alice._user_id == "alice"
+ assert s_bob._user_id == "bob"
+ finally:
+ skill_storage.reset_skill_storage()
+
+
+def test_reset_user_skill_storage_only_clears_target_user(monkeypatch):
+ """Resetting alice's storage must not invalidate bob's."""
+ skill_storage.reset_skill_storage()
+ SlowUserSkillStorage.instances_created = 0
+ _patch_user_storage_resolution(monkeypatch)
+
+ s_alice = skill_storage.get_or_new_user_skill_storage("alice")
+ s_bob = skill_storage.get_or_new_user_skill_storage("bob")
+
+ skill_storage.reset_user_skill_storage("alice")
+
+ # Alice's storage is gone
+ s_alice_new = skill_storage.get_or_new_user_skill_storage("alice")
+ assert s_alice_new is not s_alice
+
+ # Bob's is still cached
+ s_bob_cached = skill_storage.get_or_new_user_skill_storage("bob")
+ assert s_bob_cached is s_bob
+
+
+def test_reset_user_skill_storage_normalises_cache_key(monkeypatch):
+ """reset_user_skill_storage must normalise the user_id so that the cache
+ key matches the one used by get_or_new_user_skill_storage.
+
+ Without normalisation, an IM-style user ID like ``feishu:ou_xxx`` would
+ fail to clear its stale cache entry because ``get_or_new`` stores by
+ ``make_safe_user_id(user_id)`` but ``reset`` would try to pop by the raw
+ ID — a silent cache-invalidation failure.
+ """
+ from deerflow.config.paths import make_safe_user_id
+
+ skill_storage.reset_skill_storage()
+ SlowUserSkillStorage.instances_created = 0
+ _patch_user_storage_resolution(monkeypatch)
+
+ raw_id = "feishu:ou_abc123"
+ safe_id = make_safe_user_id(raw_id)
+
+ # Create storage via the normal flow (which normalises the key)
+ s = skill_storage.get_or_new_user_skill_storage(raw_id)
+ assert s._user_id == safe_id
+
+ # Reset using the raw ID — must successfully clear the cache
+ skill_storage.reset_user_skill_storage(raw_id)
+
+ # A new storage should be created (old one was evicted)
+ s_new = skill_storage.get_or_new_user_skill_storage(raw_id)
+ assert s_new is not s
diff --git a/backend/tests/test_skills_custom_router.py b/backend/tests/test_skills_custom_router.py
index 26387e5cd..7fe1b2a5e 100644
--- a/backend/tests/test_skills_custom_router.py
+++ b/backend/tests/test_skills_custom_router.py
@@ -14,7 +14,7 @@ from app.gateway.auth.models import User
from app.gateway.deps import get_config
from app.gateway.routers import skills as skills_router
from app.gateway.routers import uploads as uploads_router
-from deerflow.skills.storage import get_or_new_skill_storage
+from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
from deerflow.skills.types import Skill
@@ -73,6 +73,11 @@ def _make_skill_archive_bytes(name: str, content: str | None = None) -> bytes:
return buffer.getvalue()
+def _user_custom_dir(base_dir: Path, user_id: str = "default") -> Path:
+ """Helper to locate the per-user custom skills dir for test assertions."""
+ return base_dir / "users" / user_id / "skills" / "custom"
+
+
def test_install_skill_archive_runs_security_scan(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
(skills_root / "custom").mkdir(parents=True)
@@ -86,22 +91,29 @@ def test_install_skill_archive_runs_security_scan(monkeypatch, tmp_path):
scan_calls.append({"content": content, "executable": executable, "location": location})
return ScanResult(decision="allow", reason="ok")
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
- from types import SimpleNamespace
+ from deerflow.config.paths import Paths
- from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
+ paths = Paths(base_dir=tmp_path)
+ # Monkeypatch paths BEFORE constructing UserScopedSkillStorage,
+ # because __init__ calls get_paths() to resolve _user_custom_root.
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: paths)
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
- storage = LocalSkillStorage(host_path=str(skills_root))
+ # Use UserScopedSkillStorage so install goes to user-level dir
+ storage = UserScopedSkillStorage("default", host_path=str(skills_root))
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr(skills_router, "resolve_thread_virtual_path", lambda thread_id, path: archive)
- monkeypatch.setattr(skills_router, "get_or_new_skill_storage", lambda **kw: storage)
+ # Monkeypatch _get_user_skill_storage to return our test storage
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: storage)
monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _scan)
- monkeypatch.setattr(skills_router, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skills_router, "refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "default")
app = _make_test_app(config)
@@ -110,7 +122,9 @@ def test_install_skill_archive_runs_security_scan(monkeypatch, tmp_path):
assert response.status_code == 200
assert response.json()["skill_name"] == "archive-skill"
- assert (skills_root / "custom" / "archive-skill" / "SKILL.md").exists()
+ # UserScopedSkillStorage installs to user-level dir
+ user_custom = _user_custom_dir(tmp_path, "default")
+ assert (user_custom / "archive-skill" / "SKILL.md").exists()
assert scan_calls == [
{
"content": _skill_content("archive-skill"),
@@ -118,7 +132,7 @@ def test_install_skill_archive_runs_security_scan(monkeypatch, tmp_path):
"location": "archive-skill/SKILL.md",
}
]
- assert refresh_calls == ["refresh"]
+ assert refresh_calls == [("refresh", "default")]
def test_uploaded_skill_archive_installs_sandbox_readable_tree(monkeypatch, tmp_path):
@@ -132,8 +146,10 @@ def test_uploaded_skill_archive_installs_sandbox_readable_tree(monkeypatch, tmp_
return ScanResult(decision="allow", reason="ok")
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
+
+ from deerflow.config.paths import Paths
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
@@ -142,11 +158,18 @@ def test_uploaded_skill_archive_installs_sandbox_readable_tree(monkeypatch, tmp_
)
provider = SimpleNamespace(uses_thread_data_mounts=True)
- monkeypatch.setenv("DEER_FLOW_HOME", str(home))
+ # Monkeypatch paths BEFORE constructing UserScopedSkillStorage
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
monkeypatch.setattr("deerflow.config.paths._paths", None)
+ monkeypatch.setenv("DEER_FLOW_HOME", str(home))
monkeypatch.setattr(uploads_router, "get_sandbox_provider", lambda: provider)
monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _scan)
- monkeypatch.setattr(skills_router, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skills_router, "refresh_user_skills_system_prompt_cache_async", _refresh)
+
+ # Use UserScopedSkillStorage
+ storage = UserScopedSkillStorage("default", host_path=str(skills_root))
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "default")
app = make_authed_test_app(user_factory=_make_admin_user)
app.state.config = config
@@ -171,13 +194,13 @@ def test_uploaded_skill_archive_installs_sandbox_readable_tree(monkeypatch, tmp_
assert install_response.status_code == 200
assert install_response.json()["skill_name"] == "uploaded-skill"
- installed_dir = skills_root / "custom" / "uploaded-skill"
+ installed_dir = _user_custom_dir(tmp_path, "default") / "uploaded-skill"
nested_dir = installed_dir / "references"
assert stat.S_IMODE(installed_dir.stat().st_mode) & 0o055 == 0o055
assert stat.S_IMODE(nested_dir.stat().st_mode) & 0o055 == 0o055
assert stat.S_IMODE((installed_dir / "SKILL.md").stat().st_mode) & 0o044 == 0o044
assert stat.S_IMODE((nested_dir / "guide.md").stat().st_mode) & 0o044 == 0o044
- assert refresh_calls == ["refresh"]
+ assert refresh_calls == [("refresh", "default")]
def test_install_skill_archive_security_scan_block_returns_400(monkeypatch, tmp_path):
@@ -191,22 +214,25 @@ def test_install_skill_archive_security_scan_block_returns_400(monkeypatch, tmp_
return ScanResult(decision="block", reason="prompt injection")
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
- from types import SimpleNamespace
+ from deerflow.config.paths import Paths
- from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
+ # Monkeypatch paths BEFORE constructing UserScopedSkillStorage
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
- storage = LocalSkillStorage(host_path=str(skills_root))
+ storage = UserScopedSkillStorage("default", host_path=str(skills_root))
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr(skills_router, "resolve_thread_virtual_path", lambda thread_id, path: archive)
- monkeypatch.setattr(skills_router, "get_or_new_skill_storage", lambda **kw: storage)
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: storage)
monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _scan)
- monkeypatch.setattr(skills_router, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skills_router, "refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "default")
app = _make_test_app(config)
@@ -215,27 +241,35 @@ def test_install_skill_archive_security_scan_block_returns_400(monkeypatch, tmp_
assert response.status_code == 400
assert "Security scan blocked skill 'blocked-skill': prompt injection" in response.json()["detail"]
- assert not (skills_root / "custom" / "blocked-skill").exists()
+ assert not (_user_custom_dir(tmp_path, "default") / "blocked-skill").exists()
assert refresh_calls == []
def test_custom_skills_router_lifecycle(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- custom_dir = skills_root / "custom" / "demo-skill"
+ from deerflow.config.paths import Paths
+
+ # Create a skill in user-level custom dir
+ user_custom = _user_custom_dir(tmp_path, "default")
+ custom_dir = user_custom / "demo-skill"
custom_dir.mkdir(parents=True, exist_ok=True)
(custom_dir / "SKILL.md").write_text(_skill_content("demo-skill"), encoding="utf-8")
+
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
monkeypatch.setattr("app.gateway.routers.skills.scan_skill_content", lambda *args, **kwargs: _async_scan("allow", "ok"))
refresh_calls = []
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
- monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.get_effective_user_id", lambda: "default")
app = _make_test_app(config)
@@ -264,32 +298,42 @@ def test_custom_skills_router_lifecycle(monkeypatch, tmp_path):
assert rollback_response.status_code == 200
assert rollback_response.json()["description"] == "Demo skill"
assert stat.S_IMODE((custom_dir / "SKILL.md").stat().st_mode) & 0o044 == 0o044
- assert refresh_calls == ["refresh", "refresh"]
+ assert refresh_calls == [("refresh", "default"), ("refresh", "default")]
def test_custom_skill_rollback_blocked_by_scanner(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- custom_dir = skills_root / "custom" / "demo-skill"
+ from deerflow.config.paths import Paths
+
+ user_custom = _user_custom_dir(tmp_path, "default")
+ custom_dir = user_custom / "demo-skill"
custom_dir.mkdir(parents=True, exist_ok=True)
original_content = _skill_content("demo-skill")
edited_content = _skill_content("demo-skill", "Edited skill")
(custom_dir / "SKILL.md").write_text(edited_content, encoding="utf-8")
+
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
- history_file = get_or_new_skill_storage(app_config=config).get_skill_history_file("demo-skill")
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ # Write history file directly for the rollback test
+ storage = UserScopedSkillStorage("default", host_path=str(skills_root))
+ history_file = storage.get_skill_history_file("demo-skill")
history_file.parent.mkdir(parents=True, exist_ok=True)
history_file.write_text(
'{"action":"human_edit","prev_content":' + json.dumps(original_content) + ',"new_content":' + json.dumps(edited_content) + "}\n",
encoding="utf-8",
)
- async def _refresh():
+ async def _refresh(user_id: str):
return None
- monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.get_effective_user_id", lambda: "default")
async def _scan(*args, **kwargs):
from deerflow.skills.security_scanner import ScanResult
@@ -312,22 +356,29 @@ def test_custom_skill_rollback_blocked_by_scanner(monkeypatch, tmp_path):
def test_custom_skill_delete_preserves_history_and_allows_restore(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- custom_dir = skills_root / "custom" / "demo-skill"
+ from deerflow.config.paths import Paths
+
+ user_custom = _user_custom_dir(tmp_path, "default")
+ custom_dir = user_custom / "demo-skill"
custom_dir.mkdir(parents=True, exist_ok=True)
original_content = _skill_content("demo-skill")
(custom_dir / "SKILL.md").write_text(original_content, encoding="utf-8")
+
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
monkeypatch.setattr("app.gateway.routers.skills.scan_skill_content", lambda *args, **kwargs: _async_scan("allow", "ok"))
refresh_calls = []
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
- monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.get_effective_user_id", lambda: "default")
app = _make_test_app(config)
@@ -344,12 +395,15 @@ def test_custom_skill_delete_preserves_history_and_allows_restore(monkeypatch, t
assert rollback_response.status_code == 200
assert rollback_response.json()["description"] == "Demo skill"
assert (custom_dir / "SKILL.md").read_text(encoding="utf-8") == original_content
- assert refresh_calls == ["refresh", "refresh"]
+ assert refresh_calls == [("refresh", "default"), ("refresh", "default")]
def test_custom_skill_delete_continues_when_history_write_is_readonly(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- custom_dir = skills_root / "custom" / "demo-skill"
+ from deerflow.config.paths import Paths
+
+ user_custom = _user_custom_dir(tmp_path, "default")
+ custom_dir = user_custom / "demo-skill"
custom_dir.mkdir(parents=True, exist_ok=True)
(custom_dir / "SKILL.md").write_text(_skill_content("demo-skill"), encoding="utf-8")
config = SimpleNamespace(
@@ -357,16 +411,19 @@ def test_custom_skill_delete_continues_when_history_write_is_readonly(monkeypatc
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
refresh_calls = []
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
def _readonly_history(*args, **kwargs):
- raise OSError(errno.EROFS, "Read-only file system", str(skills_root / "custom" / ".history"))
+ raise OSError(errno.EROFS, "Read-only file system", str(user_custom / ".history"))
- monkeypatch.setattr("deerflow.skills.storage.local_skill_storage.LocalSkillStorage.append_history", _readonly_history)
- monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("deerflow.skills.storage.user_scoped_skill_storage.UserScopedSkillStorage.append_history", _readonly_history)
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.get_effective_user_id", lambda: "default")
app = _make_test_app(config)
@@ -376,12 +433,15 @@ def test_custom_skill_delete_continues_when_history_write_is_readonly(monkeypatc
assert delete_response.status_code == 200
assert delete_response.json() == {"success": True}
assert not custom_dir.exists()
- assert refresh_calls == ["refresh"]
+ assert refresh_calls == [("refresh", "default")]
def test_custom_skill_delete_fails_when_skill_dir_removal_fails(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
- custom_dir = skills_root / "custom" / "demo-skill"
+ from deerflow.config.paths import Paths
+
+ user_custom = _user_custom_dir(tmp_path, "default")
+ custom_dir = user_custom / "demo-skill"
custom_dir.mkdir(parents=True, exist_ok=True)
(custom_dir / "SKILL.md").write_text(_skill_content("demo-skill"), encoding="utf-8")
config = SimpleNamespace(
@@ -389,16 +449,19 @@ def test_custom_skill_delete_fails_when_skill_dir_removal_fails(monkeypatch, tmp
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
refresh_calls = []
- async def _refresh():
- refresh_calls.append("refresh")
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
def _fail_rmtree(*args, **kwargs):
raise PermissionError(errno.EACCES, "Permission denied", str(custom_dir))
monkeypatch.setattr("deerflow.skills.storage.local_skill_storage.shutil.rmtree", _fail_rmtree)
- monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr("app.gateway.routers.skills.get_effective_user_id", lambda: "default")
app = _make_test_app(config)
@@ -415,23 +478,63 @@ def test_update_skill_refreshes_prompt_cache_before_return(monkeypatch, tmp_path
config_path = tmp_path / "extensions_config.json"
enabled_state = {"value": True}
refresh_calls = []
+ per_user_writes: list[tuple[str, bool]] = []
def _load_skills(*, enabled_only: bool):
- skill = _make_skill("demo-skill", enabled=enabled_state["value"])
+ # Use a CUSTOM skill so the router takes the per-user cache invalidation
+ # branch (PUBLIC skills clear the cache for all users via
+ # ``clear_skills_system_prompt_cache`` — see
+ # ``test_public_skill_toggle_clears_all_users_cache``).
+ skill = Skill(
+ name="demo-skill",
+ description="Description for demo-skill",
+ license="MIT",
+ skill_dir=Path("/tmp/demo-skill"),
+ skill_file=Path("/tmp/demo-skill/SKILL.md"),
+ relative_path=Path("demo-skill"),
+ category="custom",
+ enabled=enabled_state["value"],
+ )
if enabled_only and not skill.enabled:
return []
return [skill]
- async def _refresh():
- refresh_calls.append("refresh")
- enabled_state["value"] = False
+ def _set_skill_enabled_state(name: str, enabled: bool) -> None:
+ per_user_writes.append((name, enabled))
+ enabled_state["value"] = enabled
- mock_storage = SimpleNamespace(load_skills=_load_skills)
- monkeypatch.setattr("app.gateway.routers.skills.get_or_new_skill_storage", lambda **kwargs: mock_storage)
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
+
+ # Mock storage must be a UserScopedSkillStorage instance so the
+ # router takes the per-user ``set_skill_enabled_state`` branch.
+ # We patch the symbol on the storage module because the router
+ # function imports ``UserScopedSkillStorage`` lazily from there.
+ from deerflow.skills.storage import user_scoped_skill_storage as uss_module
+
+ class _FakeUserScopedStorage:
+ def __init__(self, *args, **kwargs) -> None:
+ self._load = _load_skills
+ self._write = _set_skill_enabled_state
+
+ def load_skills(self, *, enabled_only: bool = False):
+ return self._load(enabled_only=enabled_only)
+
+ def set_skill_enabled_state(self, name: str, enabled: bool) -> None:
+ self._write(name, enabled)
+
+ monkeypatch.setattr(uss_module, "UserScopedSkillStorage", _FakeUserScopedStorage)
+ # The router also calls ``isinstance(storage, UserScopedSkillStorage)``
+ # against the symbol it imported; monkeypatch the symbol on the
+ # storage module so the isinstance check accepts our mock.
+ monkeypatch.setattr("deerflow.skills.storage.user_scoped_skill_storage.UserScopedSkillStorage", _FakeUserScopedStorage)
+ mock_storage = _FakeUserScopedStorage()
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: mock_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "default")
monkeypatch.setattr("app.gateway.routers.skills.get_extensions_config", lambda: SimpleNamespace(mcp_servers={}, skills={}))
monkeypatch.setattr("app.gateway.routers.skills.reload_extensions_config", lambda: None)
- monkeypatch.setattr(skills_router.ExtensionsConfig, "resolve_config_path", staticmethod(lambda: config_path))
- monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skills_router.ExtensionsConfig, "resolve_config_path", staticmethod(lambda config_path=None: config_path))
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _refresh)
app = _make_test_app(SimpleNamespace())
@@ -440,5 +543,382 @@ def test_update_skill_refreshes_prompt_cache_before_return(monkeypatch, tmp_path
assert response.status_code == 200
assert response.json()["enabled"] is False
- assert refresh_calls == ["refresh"]
- assert json.loads(config_path.read_text(encoding="utf-8")) == {"mcpServers": {}, "skills": {"demo-skill": {"enabled": False}}}
+ assert refresh_calls == [("refresh", "default")]
+ # CUSTOM skills write to per-user state (not extensions_config.json).
+ assert per_user_writes == [("demo-skill", False)]
+ assert not config_path.exists() or json.loads(config_path.read_text(encoding="utf-8")) == {"mcpServers": {}, "skills": {}}
+
+
+def test_public_skill_toggle_clears_all_users_cache(monkeypatch, tmp_path):
+ """P2-5: toggling a PUBLIC skill must invalidate the prompt cache for
+ every user, because PUBLIC state lives in the global
+ ``extensions_config.json`` and a per-user ``refresh_*`` call would
+ leave the other users' cached enabled state stale.
+ """
+ config_path = tmp_path / "extensions_config.json"
+ config_path.write_text(json.dumps({"mcpServers": {}, "skills": {"public-skill": {"enabled": True}}}), encoding="utf-8")
+ clear_calls = []
+ refresh_calls = []
+ load_calls = {"n": 0}
+
+ def _load_skills(*, enabled_only: bool):
+ from deerflow.config.extensions_config import ExtensionsConfig
+ from deerflow.skills.types import Skill
+
+ # The router re-loads after the toggle so the response reflects
+ # the new state. The second call therefore reads the on-disk
+ # JSON, which the PUBLIC branch has just rewritten.
+ load_calls["n"] += 1
+ if load_calls["n"] >= 2:
+ on_disk = ExtensionsConfig.from_file(config_path)
+ current_enabled = on_disk.skills["public-skill"].enabled
+ else:
+ current_enabled = True
+
+ skill = Skill(
+ name="public-skill",
+ description="Description for public-skill",
+ license="MIT",
+ skill_dir=Path("/tmp/public-skill"),
+ skill_file=Path("/tmp/public-skill/SKILL.md"),
+ relative_path=Path("public-skill"),
+ category="public",
+ enabled=current_enabled,
+ )
+ if enabled_only and not skill.enabled:
+ return []
+ return [skill]
+
+ def _clear():
+ clear_calls.append("clear")
+
+ async def _refresh(user_id: str):
+ refresh_calls.append(("refresh", user_id))
+
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: SimpleNamespace(load_skills=_load_skills))
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "default")
+ # ``resolve_config_path()`` is called with no args inside the router
+ # to discover where to write; point it at the temp file.
+ monkeypatch.setattr(skills_router.ExtensionsConfig, "resolve_config_path", staticmethod(lambda config_path=None: config_path if config_path is not None else config_path))
+
+ # Re-bind the staticmethod to actually default to config_path when
+ # called with no args. ``staticmethod`` is a descriptor, so we wrap
+ # with a callable that always returns the test path.
+ def _resolve(_config_path=None):
+ return config_path
+
+ monkeypatch.setattr(skills_router.ExtensionsConfig, "resolve_config_path", staticmethod(_resolve))
+ monkeypatch.setattr("app.gateway.routers.skills.get_extensions_config", lambda: __import__("deerflow.config.extensions_config", fromlist=["ExtensionsConfig"]).ExtensionsConfig.from_file(config_path))
+ monkeypatch.setattr("app.gateway.routers.skills.reload_extensions_config", lambda: None)
+ monkeypatch.setattr("app.gateway.routers.skills.clear_skills_system_prompt_cache", _clear)
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _refresh)
+
+ app = _make_test_app(SimpleNamespace())
+
+ with TestClient(app) as client:
+ response = client.put("/api/skills/public-skill", json={"enabled": False})
+
+ assert response.status_code == 200, response.text
+ assert response.json()["enabled"] is False
+ # PUBLIC skills must hit the global cache-clear branch, not per-user refresh.
+ assert clear_calls == ["clear"]
+ assert refresh_calls == []
+ # The global state file must reflect the toggle.
+ persisted = json.loads(config_path.read_text(encoding="utf-8"))
+ assert persisted["skills"]["public-skill"]["enabled"] is False
+
+
+class TestMultiUserSkillIsolation:
+ """End-to-end integration tests verifying per-user skill isolation
+ through the HTTP router → _get_user_skill_storage → filesystem chain.
+
+ These tests simulate two distinct users (alice and bob) calling the
+ same API endpoints and verify that each user's skills are completely
+ isolated: alice cannot see/edit/delete bob's skills and vice versa.
+ """
+
+ @staticmethod
+ def _setup_two_user_env(monkeypatch, tmp_path, skills_root):
+ """Shared setup: patch paths and create two UserScopedSkillStorage instances."""
+ from deerflow.config.paths import Paths
+
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ alice_storage = UserScopedSkillStorage("alice", host_path=str(skills_root))
+ bob_storage = UserScopedSkillStorage("bob", host_path=str(skills_root))
+
+ config = SimpleNamespace(
+ skills=SimpleNamespace(
+ get_skills_path=lambda: skills_root,
+ container_path="/mnt/skills",
+ use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
+ ),
+ skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
+ )
+ return alice_storage, bob_storage, config
+
+ def test_alice_skill_not_visible_to_bob_via_list_api(self, monkeypatch, tmp_path):
+ """Alice installs a skill; Bob's /api/skills listing does not include it."""
+ skills_root = tmp_path / "skills"
+ skills_root.mkdir()
+ alice_storage, bob_storage, config = self._setup_two_user_env(monkeypatch, tmp_path, skills_root)
+
+ # Alice creates a skill directly in her per-user directory
+ alice_custom = _user_custom_dir(tmp_path, "alice")
+ skill_dir = alice_custom / "alice-secret-skill"
+ skill_dir.mkdir(parents=True, exist_ok=True)
+ (skill_dir / "SKILL.md").write_text(_skill_content("alice-secret-skill"), encoding="utf-8")
+
+ # Alice's listing includes her skill
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: alice_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "alice")
+ alice_app = _make_test_app(config)
+
+ with TestClient(alice_app) as client:
+ alice_response = client.get("/api/skills")
+ alice_skills = alice_response.json()["skills"]
+ alice_custom_skills = [s for s in alice_skills if s["category"] == "custom"]
+ assert len(alice_custom_skills) == 1
+ assert alice_custom_skills[0]["name"] == "alice-secret-skill"
+
+ # Bob's listing does NOT include Alice's skill
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: bob_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "bob")
+ bob_app = _make_test_app(config)
+
+ with TestClient(bob_app) as client:
+ bob_response = client.get("/api/skills")
+ bob_skills = bob_response.json()["skills"]
+ bob_custom_skills = [s for s in bob_skills if s["category"] == "custom"]
+ assert len(bob_custom_skills) == 0
+
+ def test_bob_cannot_read_alice_skill_via_get_api(self, monkeypatch, tmp_path):
+ """Bob cannot GET /api/skills/custom/alice-secret-skill — 404."""
+ skills_root = tmp_path / "skills"
+ skills_root.mkdir()
+ alice_storage, _, config = self._setup_two_user_env(monkeypatch, tmp_path, skills_root)
+
+ alice_custom = _user_custom_dir(tmp_path, "alice")
+ skill_dir = alice_custom / "alice-secret-skill"
+ skill_dir.mkdir(parents=True, exist_ok=True)
+ (skill_dir / "SKILL.md").write_text(_skill_content("alice-secret-skill"), encoding="utf-8")
+
+ # Simulate Bob's request context
+ bob_storage = UserScopedSkillStorage("bob", host_path=str(skills_root))
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: bob_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "bob")
+ bob_app = _make_test_app(config)
+
+ with TestClient(bob_app) as client:
+ bob_get_response = client.get("/api/skills/custom/alice-secret-skill")
+ assert bob_get_response.status_code == 404
+
+ # Alice can still read it
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: alice_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "alice")
+ alice_app = _make_test_app(config)
+
+ with TestClient(alice_app) as client:
+ alice_get_response = client.get("/api/skills/custom/alice-secret-skill")
+ assert alice_get_response.status_code == 200
+ assert "# alice-secret-skill" in alice_get_response.json()["content"]
+
+ def test_bob_cannot_delete_alice_skill(self, monkeypatch, tmp_path):
+ """Bob cannot DELETE /api/skills/custom/alice-secret-skill — 404."""
+ skills_root = tmp_path / "skills"
+ skills_root.mkdir()
+ alice_storage, _, config = self._setup_two_user_env(monkeypatch, tmp_path, skills_root)
+
+ alice_custom = _user_custom_dir(tmp_path, "alice")
+ skill_dir = alice_custom / "alice-secret-skill"
+ skill_dir.mkdir(parents=True, exist_ok=True)
+ (skill_dir / "SKILL.md").write_text(_skill_content("alice-secret-skill"), encoding="utf-8")
+
+ bob_storage = UserScopedSkillStorage("bob", host_path=str(skills_root))
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: bob_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "bob")
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", lambda uid: None)
+ bob_app = _make_test_app(config)
+
+ with TestClient(bob_app) as client:
+ bob_delete_response = client.delete("/api/skills/custom/alice-secret-skill")
+ assert bob_delete_response.status_code == 404
+
+ # Alice's skill file still exists
+ assert (skill_dir / "SKILL.md").exists()
+
+ def test_alice_cannot_edit_bob_skill_via_update_api(self, monkeypatch, tmp_path):
+ """Alice cannot PUT /api/skills/custom/bob-skill — 404."""
+ skills_root = tmp_path / "skills"
+ skills_root.mkdir()
+ _, bob_storage, config = self._setup_two_user_env(monkeypatch, tmp_path, skills_root)
+
+ bob_custom = _user_custom_dir(tmp_path, "bob")
+ skill_dir = bob_custom / "bob-priv-skill"
+ skill_dir.mkdir(parents=True, exist_ok=True)
+ (skill_dir / "SKILL.md").write_text(_skill_content("bob-priv-skill"), encoding="utf-8")
+
+ alice_storage = UserScopedSkillStorage("alice", host_path=str(skills_root))
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: alice_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "alice")
+ monkeypatch.setattr("app.gateway.routers.skills.scan_skill_content", lambda *a, **kw: _async_scan("allow", "ok"))
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", lambda uid: None)
+ alice_app = _make_test_app(config)
+
+ with TestClient(alice_app) as client:
+ alice_update_response = client.put(
+ "/api/skills/custom/bob-priv-skill",
+ json={"content": _skill_content("bob-priv-skill", "Hacked by alice")},
+ )
+ assert alice_update_response.status_code == 404
+
+ # Bob's skill content is unchanged
+ assert (skill_dir / "SKILL.md").read_text(encoding="utf-8") == _skill_content("bob-priv-skill")
+
+ def test_two_users_install_same_skill_name_independently(self, monkeypatch, tmp_path):
+ """Both users install a skill named 'my-workflow' — they are stored separately."""
+ skills_root = tmp_path / "skills"
+ skills_root.mkdir()
+ alice_storage, bob_storage, config = self._setup_two_user_env(monkeypatch, tmp_path, skills_root)
+
+ # Both users create a skill with the same name but different content
+ alice_custom = _user_custom_dir(tmp_path, "alice")
+ alice_dir = alice_custom / "my-workflow"
+ alice_dir.mkdir(parents=True, exist_ok=True)
+ (alice_dir / "SKILL.md").write_text(_skill_content("my-workflow", "Alice's version"), encoding="utf-8")
+
+ bob_custom = _user_custom_dir(tmp_path, "bob")
+ bob_dir = bob_custom / "my-workflow"
+ bob_dir.mkdir(parents=True, exist_ok=True)
+ (bob_dir / "SKILL.md").write_text(_skill_content("my-workflow", "Bob's version"), encoding="utf-8")
+
+ # Alice sees her version
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: alice_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "alice")
+ alice_app = _make_test_app(config)
+
+ with TestClient(alice_app) as client:
+ alice_skills = client.get("/api/skills").json()["skills"]
+ alice_custom_skills = [s for s in alice_skills if s["category"] == "custom"]
+ assert len(alice_custom_skills) == 1
+ assert alice_custom_skills[0]["name"] == "my-workflow"
+
+ alice_content = client.get("/api/skills/custom/my-workflow").json()["content"]
+ assert "Alice's version" in alice_content
+
+ # Bob sees his version
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: bob_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "bob")
+ bob_app = _make_test_app(config)
+
+ with TestClient(bob_app) as client:
+ bob_skills = client.get("/api/skills").json()["skills"]
+ bob_custom_skills = [s for s in bob_skills if s["category"] == "custom"]
+ assert len(bob_custom_skills) == 1
+ assert bob_custom_skills[0]["name"] == "my-workflow"
+
+ bob_content = client.get("/api/skills/custom/my-workflow").json()["content"]
+ assert "Bob's version" in bob_content
+
+ def test_skill_response_includes_editable_field(self, monkeypatch, tmp_path):
+ """SkillResponse.editable is true for CUSTOM, false for PUBLIC and LEGACY."""
+ skills_root = tmp_path / "skills"
+ skills_root.mkdir()
+
+ # Create a public skill
+ public_dir = skills_root / "public" / "deep-research"
+ public_dir.mkdir(parents=True, exist_ok=True)
+ (public_dir / "SKILL.md").write_text(_skill_content("deep-research", "Built-in skill"), encoding="utf-8")
+
+ # Create a global custom skill (LEGACY fallback for users without per-user dir)
+ global_custom_dir = skills_root / "custom" / "legacy-shared-skill"
+ global_custom_dir.mkdir(parents=True, exist_ok=True)
+ (global_custom_dir / "SKILL.md").write_text(_skill_content("legacy-shared-skill", "Legacy shared skill"), encoding="utf-8")
+
+ # Create a per-user custom skill
+ from deerflow.config.paths import Paths
+
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ alice_storage = UserScopedSkillStorage("alice", host_path=str(skills_root))
+ alice_custom = _user_custom_dir(tmp_path, "alice")
+ alice_dir = alice_custom / "alice-custom-skill"
+ alice_dir.mkdir(parents=True, exist_ok=True)
+ (alice_dir / "SKILL.md").write_text(_skill_content("alice-custom-skill"), encoding="utf-8")
+
+ config = SimpleNamespace(
+ skills=SimpleNamespace(
+ get_skills_path=lambda: skills_root,
+ container_path="/mnt/skills",
+ use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
+ ),
+ skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
+ )
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: alice_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "alice")
+ monkeypatch.setattr("app.gateway.routers.skills.get_extensions_config", lambda: SimpleNamespace(mcp_servers={}, skills={}))
+ monkeypatch.setattr("app.gateway.routers.skills.reload_extensions_config", lambda: None)
+
+ app = _make_test_app(config)
+
+ with TestClient(app) as client:
+ response = client.get("/api/skills")
+ skills = response.json()["skills"]
+
+ # PUBLIC skill: editable=false
+ public_skill = next(s for s in skills if s["name"] == "deep-research")
+ assert public_skill["category"] == "public"
+ assert public_skill["editable"] is False
+
+ # CUSTOM skill: editable=true
+ custom_skill = next(s for s in skills if s["name"] == "alice-custom-skill")
+ assert custom_skill["category"] == "custom"
+ assert custom_skill["editable"] is True
+
+ def test_toggle_enabled_accepted_for_custom_skill(self, monkeypatch, tmp_path):
+ """PUT /api/skills/ with {enabled: false} returns 200.
+
+ All skill categories (public, custom, legacy) can be toggled via
+ extensions_config. CUSTOM skills default to enabled, but users may
+ disable them temporarily without deleting.
+ """
+ skills_root = tmp_path / "skills"
+ skills_root.mkdir()
+
+ from deerflow.config.paths import Paths
+
+ monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+ monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+ alice_storage = UserScopedSkillStorage("alice", host_path=str(skills_root))
+ alice_custom = _user_custom_dir(tmp_path, "alice")
+ alice_dir = alice_custom / "alice-custom-skill"
+ alice_dir.mkdir(parents=True, exist_ok=True)
+ (alice_dir / "SKILL.md").write_text(_skill_content("alice-custom-skill"), encoding="utf-8")
+
+ config = SimpleNamespace(
+ skills=SimpleNamespace(
+ get_skills_path=lambda: skills_root,
+ container_path="/mnt/skills",
+ use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
+ ),
+ skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
+ )
+
+ async def _noop_async(user_id: str) -> None:
+ pass
+
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: alice_storage)
+ monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "alice")
+ monkeypatch.setattr("app.gateway.routers.skills.get_extensions_config", lambda: SimpleNamespace(mcp_servers={}, skills={}))
+ monkeypatch.setattr("app.gateway.routers.skills.reload_extensions_config", lambda: None)
+ monkeypatch.setattr("app.gateway.routers.skills.refresh_user_skills_system_prompt_cache_async", _noop_async)
+
+ app = _make_test_app(config)
+
+ with TestClient(app) as client:
+ response = client.put("/api/skills/alice-custom-skill", json={"enabled": False})
+ assert response.status_code == 200
+ assert response.json()["enabled"] is False
diff --git a/backend/tests/test_skills_router_authz.py b/backend/tests/test_skills_router_authz.py
index b738b357b..9e44afdea 100644
--- a/backend/tests/test_skills_router_authz.py
+++ b/backend/tests/test_skills_router_authz.py
@@ -1,10 +1,17 @@
"""Authorization regression tests for the skills router.
-Skills storage is global/shared across all users, and custom skill SKILL.md
-content is injected into every user's agent system prompt. The mutating skills
-endpoints (and the endpoints that expose raw custom-skill content/history) must
-therefore be admin-only, matching the MCP router which guards the equivalent
-global extensions_config mutations with ``require_admin_user``.
+Custom skill SKILL.md content is injected into every user's agent system
+prompt. The mutating endpoints that write global shared state (install,
+toggle PUBLIC skills, edit/delete custom skill content, and the endpoints
+that expose raw custom-skill content/history) must be admin-only, matching
+the MCP router which guards the equivalent global extensions_config mutations
+with ``require_admin_user``.
+
+Under per-user skill isolation, ``list_custom_skills`` is open to all
+authenticated users (they see only their own custom skills), but all other
+custom-skill endpoints remain admin-only because they write global state
+(install writes to the shared archive, toggle writes extensions_config.json
+for PUBLIC skills, and edit/delete modify the on-disk skill tree).
These tests pin the access-control boundary: a normal authenticated
(non-admin) user must receive 403 on every guarded endpoint.
@@ -41,13 +48,14 @@ def _make_app(*, system_role: str) -> FastAPI:
# (method, path, json_body) for every endpoint that must require admin.
-# Every entry here writes/reads global shared state (the custom skills tree,
-# the shared extensions_config.json, or raw global skill content), so all are
-# admin-only. PUT /api/skills/{name} is included: toggling enabled writes the
-# shared extensions_config.json and changes every tenant's injected skill set.
+# Under per-user skill isolation, list_custom_skills is open to normal users
+# (they see only their own skills), so it is NOT in this list.
+# All other mutating endpoints write/read global shared state and must be
+# admin-only. PUT /api/skills/{name} is included: toggling enabled writes
+# the shared extensions_config.json (for PUBLIC skills) and changes every
+# tenant's injected skill set.
_GUARDED_ENDPOINTS = [
("post", "/api/skills/install", {"thread_id": "t1", "path": "mnt/user-data/outputs/x.skill"}),
- ("get", "/api/skills/custom", None),
("get", "/api/skills/custom/demo", None),
("put", "/api/skills/custom/demo", {"content": "---\nname: demo\ndescription: hijacked\n---\n"}),
("delete", "/api/skills/custom/demo", None),
@@ -74,6 +82,9 @@ def test_non_admin_is_forbidden_on_all_mutating_skills_endpoints():
def test_basic_skill_listing_stays_open_to_normal_users(monkeypatch):
"""The basic list/detail endpoints expose only name/description and are
needed by the normal-user UI, so they must NOT be admin-gated.
+
+ Under per-user skill isolation, ``list_custom_skills`` (GET /api/skills/custom)
+ is also open to normal users — they see only their own custom skills.
"""
def _load_skills(*, enabled_only: bool):
@@ -96,9 +107,10 @@ def test_basic_skill_listing_stays_open_to_normal_users(monkeypatch):
app = _make_app(system_role="user")
app.dependency_overrides[get_config] = lambda: SimpleNamespace()
- monkeypatch.setattr(skills_router, "get_or_new_skill_storage", lambda **kw: SimpleNamespace(load_skills=_load_skills))
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: SimpleNamespace(load_skills=_load_skills))
with TestClient(app) as client:
assert client.get("/api/skills").status_code == 200
+ assert client.get("/api/skills/custom").status_code == 200
assert client.get("/api/skills/demo").status_code == 200
@@ -127,15 +139,15 @@ def test_enable_toggle_allowed_for_admin(monkeypatch, tmp_path):
]
app = _make_app(system_role="admin")
- monkeypatch.setattr(skills_router, "get_or_new_skill_storage", lambda **kw: SimpleNamespace(load_skills=_load_skills))
+ monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: SimpleNamespace(load_skills=_load_skills))
monkeypatch.setattr(skills_router, "get_extensions_config", lambda: SimpleNamespace(mcp_servers={}, skills={}))
monkeypatch.setattr(skills_router, "reload_extensions_config", lambda: None)
monkeypatch.setattr(skills_router.ExtensionsConfig, "resolve_config_path", staticmethod(lambda: config_path))
- async def _refresh():
+ async def _refresh(_user_id: str):
return None
- monkeypatch.setattr(skills_router, "refresh_skills_system_prompt_cache_async", _refresh)
+ monkeypatch.setattr(skills_router, "refresh_user_skills_system_prompt_cache_async", _refresh)
with TestClient(app) as client:
resp = client.put("/api/skills/demo", json={"enabled": False})
assert resp.status_code == 200, f"admin toggle should succeed, got {resp.status_code}"
diff --git a/backend/tests/test_slash_skills.py b/backend/tests/test_slash_skills.py
index ec2e51968..6afa749f3 100644
--- a/backend/tests/test_slash_skills.py
+++ b/backend/tests/test_slash_skills.py
@@ -32,10 +32,20 @@ def _make_skill(tmp_path: Path, name: str, content: str = "skill body") -> Skill
def _make_storage(tmp_path: Path, skills: list[Skill]):
+ def _validate_skill_file_path(skill_file: Path) -> Path:
+ resolved = skill_file.resolve()
+ root = tmp_path.resolve()
+ try:
+ resolved.relative_to(root)
+ except ValueError:
+ raise ValueError("Resolved skill file must stay within the configured skills root.")
+ return resolved
+
return SimpleNamespace(
load_skills=lambda *, enabled_only: [skill for skill in skills if skill.enabled] if enabled_only else skills,
get_container_root=lambda: "/mnt/skills",
get_skills_root_path=lambda: tmp_path,
+ validate_skill_file_path=_validate_skill_file_path,
)
diff --git a/backend/tests/test_user_scoped_skill_storage.py b/backend/tests/test_user_scoped_skill_storage.py
new file mode 100644
index 000000000..47b124a89
--- /dev/null
+++ b/backend/tests/test_user_scoped_skill_storage.py
@@ -0,0 +1,573 @@
+"""Tests for UserScopedSkillStorage: per-user isolation, fallback, and path safety."""
+
+from __future__ import annotations
+
+import stat
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+from deerflow.config.paths import Paths
+from deerflow.skills.storage import reset_skill_storage, reset_user_skill_storage
+from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
+from deerflow.skills.types import SkillCategory
+
+
+def _skill_content(name: str, description: str = "Demo skill") -> str:
+ return f"---\nname: {name}\ndescription: {description}\n---\n\n# {name}\n"
+
+
+@pytest.fixture(autouse=True)
+def _reset_storages():
+ """Reset all skill storage caches between tests."""
+ reset_skill_storage()
+ yield
+ reset_skill_storage()
+
+
+@pytest.fixture
+def base_dir(tmp_path: Path) -> Path:
+ """Provide a temp directory as the DeerFlow base_dir."""
+ return tmp_path
+
+
+@pytest.fixture
+def paths(base_dir: Path) -> Paths:
+ return Paths(base_dir=base_dir)
+
+
+@pytest.fixture
+def skills_root(base_dir: Path) -> Path:
+ """Create the global skills root directory with public/ and custom/ subdirs."""
+ root = base_dir / "skills"
+ root.mkdir()
+ (root / "public").mkdir()
+ (root / "custom").mkdir()
+ return root
+
+
+@pytest.fixture
+def config(skills_root):
+ """Minimal app_config-like namespace for storage construction."""
+ from types import SimpleNamespace
+
+ return SimpleNamespace(
+ skills=SimpleNamespace(
+ get_skills_path=lambda: skills_root,
+ container_path="/mnt/skills",
+ use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
+ ),
+ )
+
+
+@pytest.fixture
+def user_storage(base_dir: Path, skills_root, config) -> UserScopedSkillStorage:
+ """Create a UserScopedSkillStorage for user 'test-user'."""
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ storage = UserScopedSkillStorage("test-user", host_path=str(skills_root), app_config=config)
+ return storage
+
+
+class TestPathRedirection:
+ """Custom skill paths are redirected to per-user directories."""
+
+ def test_custom_skill_dir_is_user_scoped(self, user_storage: UserScopedSkillStorage, base_dir: Path):
+ expected = base_dir / "users" / "test-user" / "skills" / "custom" / "demo-skill"
+ assert user_storage.get_custom_skill_dir("demo-skill") == expected
+
+ def test_custom_skill_file_is_user_scoped(self, user_storage: UserScopedSkillStorage, base_dir: Path):
+ expected = base_dir / "users" / "test-user" / "skills" / "custom" / "demo-skill" / "SKILL.md"
+ assert user_storage.get_custom_skill_file("demo-skill") == expected
+
+ def test_history_file_is_user_scoped(self, user_storage: UserScopedSkillStorage, base_dir: Path):
+ expected = base_dir / "users" / "test-user" / "skills" / "custom" / ".history" / "demo-skill.jsonl"
+ assert user_storage.get_skill_history_file("demo-skill") == expected
+
+ def test_public_skill_paths_still_use_global_root(self, user_storage: UserScopedSkillStorage, skills_root: Path):
+ assert user_storage.get_skills_root_path() == skills_root
+
+ def test_user_id_property(self, user_storage: UserScopedSkillStorage):
+ assert user_storage.user_id == "test-user"
+
+
+class TestWriteAndRead:
+ """Writes go to user dir, reads from user dir when present."""
+
+ def test_write_creates_file_in_user_dir(self, user_storage: UserScopedSkillStorage, base_dir: Path):
+ user_storage.write_custom_skill("demo-skill", "SKILL.md", _skill_content("demo-skill"))
+ user_file = base_dir / "users" / "test-user" / "skills" / "custom" / "demo-skill" / "SKILL.md"
+ assert user_file.exists()
+ assert user_file.read_text(encoding="utf-8") == _skill_content("demo-skill")
+
+ def test_write_does_not_create_in_global_custom(self, user_storage: UserScopedSkillStorage, skills_root: Path, base_dir: Path):
+ user_storage.write_custom_skill("demo-skill", "SKILL.md", _skill_content("demo-skill"))
+ global_file = skills_root / "custom" / "demo-skill" / "SKILL.md"
+ assert not global_file.exists()
+
+ def test_read_from_user_dir(self, user_storage: UserScopedSkillStorage, base_dir: Path):
+ user_storage.write_custom_skill("demo-skill", "SKILL.md", _skill_content("demo-skill"))
+ content = user_storage.read_custom_skill("demo-skill")
+ assert "demo-skill" in content
+
+ def test_read_not_found_raises(self, user_storage: UserScopedSkillStorage):
+ with pytest.raises(FileNotFoundError):
+ user_storage.read_custom_skill("nonexistent")
+
+ def test_write_makes_path_sandbox_readable(self, user_storage: UserScopedSkillStorage, base_dir: Path):
+ user_storage.write_custom_skill("demo-skill", "references/ref.md", "# ref")
+ skill_dir = base_dir / "users" / "test-user" / "skills" / "custom" / "demo-skill"
+ ref_dir = skill_dir / "references"
+ assert stat.S_IMODE(skill_dir.stat().st_mode) & 0o055 == 0o055
+ assert stat.S_IMODE(ref_dir.stat().st_mode) & 0o055 == 0o055
+
+
+class TestSkillLoading:
+ """Public skills from global, custom from user dir + fallback."""
+
+ def test_public_skills_loaded_from_global(self, user_storage: UserScopedSkillStorage, skills_root: Path):
+ public_dir = skills_root / "public" / "deep-research"
+ public_dir.mkdir(parents=True)
+ (public_dir / "SKILL.md").write_text(_skill_content("deep-research"), encoding="utf-8")
+
+ skills = user_storage.load_skills(enabled_only=False)
+ public_skills = [s for s in skills if s.category == SkillCategory.PUBLIC]
+ assert len(public_skills) == 1
+ assert public_skills[0].name == "deep-research"
+
+ def test_custom_skills_loaded_from_user_dir(self, user_storage: UserScopedSkillStorage, base_dir: Path):
+ user_storage.write_custom_skill("my-skill", "SKILL.md", _skill_content("my-skill"))
+
+ skills = user_storage.load_skills(enabled_only=False)
+ custom_skills = [s for s in skills if s.category == SkillCategory.CUSTOM]
+ assert len(custom_skills) == 1
+ assert custom_skills[0].name == "my-skill"
+
+ def test_fallback_to_global_custom_when_user_dir_empty(self, user_storage: UserScopedSkillStorage, skills_root: Path, base_dir: Path):
+ # Put skill in global custom (NOT in user dir)
+ global_dir = skills_root / "custom" / "global-skill"
+ global_dir.mkdir(parents=True)
+ (global_dir / "SKILL.md").write_text(_skill_content("global-skill"), encoding="utf-8")
+
+ # User dir is empty → fallback loads from global custom as LEGACY
+ skills = user_storage.load_skills(enabled_only=False)
+ legacy_skills = [s for s in skills if s.category == SkillCategory.LEGACY]
+ assert len(legacy_skills) == 1
+ assert legacy_skills[0].name == "global-skill"
+
+ def test_no_fallback_when_user_dir_has_content(self, user_storage: UserScopedSkillStorage, skills_root: Path, base_dir: Path):
+ # Put skill in global custom
+ global_dir = skills_root / "custom" / "global-skill"
+ global_dir.mkdir(parents=True)
+ (global_dir / "SKILL.md").write_text(_skill_content("global-skill"), encoding="utf-8")
+
+ # Also put skill in user custom
+ user_storage.write_custom_skill("user-skill", "SKILL.md", _skill_content("user-skill"))
+
+ # User dir has content → no fallback, only user-level skill
+ skills = user_storage.load_skills(enabled_only=False)
+ custom_skills = [s for s in skills if s.category == SkillCategory.CUSTOM]
+ assert len(custom_skills) == 1
+ assert custom_skills[0].name == "user-skill"
+
+ def test_mixed_public_and_custom(self, user_storage: UserScopedSkillStorage, skills_root: Path, base_dir: Path):
+ # Create public skill
+ public_dir = skills_root / "public" / "deep-research"
+ public_dir.mkdir(parents=True)
+ (public_dir / "SKILL.md").write_text(_skill_content("deep-research"), encoding="utf-8")
+
+ # Create user custom skill
+ user_storage.write_custom_skill("my-skill", "SKILL.md", _skill_content("my-skill"))
+
+ skills = user_storage.load_skills(enabled_only=False)
+ assert len(skills) == 2
+ categories = {s.category for s in skills}
+ assert categories == {SkillCategory.PUBLIC, SkillCategory.CUSTOM}
+
+
+class TestIsolation:
+ """Different users must see different custom skills."""
+
+ def test_two_users_isolated(self, base_dir: Path, skills_root, config):
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ storage_a = UserScopedSkillStorage("alice", host_path=str(skills_root), app_config=config)
+ storage_b = UserScopedSkillStorage("bob", host_path=str(skills_root), app_config=config)
+
+ storage_a.write_custom_skill("skill-a", "SKILL.md", _skill_content("skill-a"))
+ storage_b.write_custom_skill("skill-b", "SKILL.md", _skill_content("skill-b"))
+
+ skills_a = [s for s in storage_a.load_skills(enabled_only=False) if s.category == SkillCategory.CUSTOM]
+ skills_b = [s for s in storage_b.load_skills(enabled_only=False) if s.category == SkillCategory.CUSTOM]
+
+ assert len(skills_a) == 1
+ assert skills_a[0].name == "skill-a"
+ assert len(skills_b) == 1
+ assert skills_b[0].name == "skill-b"
+
+ def test_delete_is_isolated(self, base_dir: Path, skills_root, config):
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ storage_a = UserScopedSkillStorage("alice", host_path=str(skills_root), app_config=config)
+ storage_b = UserScopedSkillStorage("bob", host_path=str(skills_root), app_config=config)
+
+ storage_a.write_custom_skill("skill-a", "SKILL.md", _skill_content("skill-a"))
+ storage_b.write_custom_skill("skill-b", "SKILL.md", _skill_content("skill-b"))
+
+ storage_a.delete_custom_skill("skill-a")
+
+ # Alice has no custom skills, Bob still has theirs
+ skills_a = [s for s in storage_a.load_skills(enabled_only=False) if s.category == SkillCategory.CUSTOM]
+ skills_b = [s for s in storage_b.load_skills(enabled_only=False) if s.category == SkillCategory.CUSTOM]
+
+ assert len(skills_a) == 0
+ assert len(skills_b) == 1
+
+
+class TestHistoryIsolation:
+ """History files are per-user."""
+
+ def test_history_per_user(self, base_dir: Path, skills_root, config):
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ storage_a = UserScopedSkillStorage("alice", host_path=str(skills_root), app_config=config)
+ storage_a.write_custom_skill("shared-name", "SKILL.md", _skill_content("shared-name"))
+
+ storage_a.append_history("shared-name", {"action": "create", "author": "alice"})
+
+ history_file_a = base_dir / "users" / "alice" / "skills" / "custom" / ".history" / "shared-name.jsonl"
+ assert history_file_a.exists()
+
+ def test_history_does_not_leak_to_global(self, base_dir: Path, skills_root, config):
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ storage = UserScopedSkillStorage("alice", host_path=str(skills_root), app_config=config)
+ storage.write_custom_skill("my-skill", "SKILL.md", _skill_content("my-skill"))
+ storage.append_history("my-skill", {"action": "create"})
+
+ global_history = skills_root / "custom" / ".history" / "my-skill.jsonl"
+ assert not global_history.exists()
+
+
+class TestPathSafety:
+ """UserScopedSkillStorage inherits path-traversal guards from LocalSkillStorage."""
+
+ def test_rejects_invalid_skill_name(self, user_storage: UserScopedSkillStorage):
+ with pytest.raises(ValueError, match="hyphen-case"):
+ user_storage.get_custom_skill_dir("../../escaped")
+
+ def test_rejects_path_traversal_in_write(self, user_storage: UserScopedSkillStorage):
+ with pytest.raises(ValueError, match="skill directory"):
+ user_storage.write_custom_skill("demo-skill", "../../escaped.txt", "x")
+
+ def test_rejects_empty_path_in_write(self, user_storage: UserScopedSkillStorage):
+ with pytest.raises(ValueError, match="empty"):
+ user_storage.write_custom_skill("demo-skill", "", "x")
+
+
+class TestFactory:
+ """get_or_new_user_skill_storage factory behavior."""
+
+ def test_returns_same_instance_for_same_user(self, base_dir: Path, skills_root, config):
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ from deerflow.skills.storage import get_or_new_user_skill_storage
+
+ s1 = get_or_new_user_skill_storage("alice", app_config=config)
+ s2 = get_or_new_user_skill_storage("alice", app_config=config)
+ assert s1 is s2
+
+ def test_returns_different_instance_for_different_user(self, base_dir: Path, skills_root, config):
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ from deerflow.skills.storage import get_or_new_user_skill_storage
+
+ s1 = get_or_new_user_skill_storage("alice", app_config=config)
+ s2 = get_or_new_user_skill_storage("bob", app_config=config)
+ assert s1 is not s2
+
+ def test_reset_clears_specific_user(self, base_dir: Path, skills_root, config):
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ from deerflow.skills.storage import get_or_new_user_skill_storage
+
+ s_alice = get_or_new_user_skill_storage("alice", app_config=config)
+ s_bob = get_or_new_user_skill_storage("bob", app_config=config)
+
+ reset_user_skill_storage("alice")
+
+ # Alice's storage is gone; a new one is created
+ s_alice_new = get_or_new_user_skill_storage("alice", app_config=config)
+ assert s_alice_new is not s_alice
+
+ # Bob's storage is still cached
+ s_bob_cached = get_or_new_user_skill_storage("bob", app_config=config)
+ assert s_bob_cached is s_bob
+
+
+class TestSkillToggleIsolation:
+ """Per-user enabled/disabled state isolation for same-named custom skills.
+
+ When Alice and Bob each own a custom skill named 'report-gen', disabling
+ Alice's copy must NOT affect Bob's. The enabled state is stored in
+ per-user ``_skill_states.json`` so same-named skills can be toggled
+ independently across users.
+ """
+
+ def test_alice_disable_does_not_affect_bob(self, base_dir: Path, skills_root, config):
+ from types import SimpleNamespace
+
+ from deerflow.agents.lead_agent.prompt import clear_skills_system_prompt_cache, get_skills_prompt_section
+ from deerflow.sandbox.tools import _is_disabled_skill_path
+ from deerflow.skills.storage import get_or_new_user_skill_storage
+
+ # Rich config that includes skill_evolution (required by
+ # get_skills_prompt_section) while keeping the test skills root.
+ rich_config = SimpleNamespace(
+ skills=config.skills,
+ skill_evolution=SimpleNamespace(enabled=False),
+ )
+
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base_dir)):
+ with patch("deerflow.config.paths._paths", None):
+ with patch("deerflow.config.get_app_config", return_value=rich_config):
+ # Use the factory so storages enter the cache — both
+ # _is_disabled_skill_path and get_skills_prompt_section
+ # call the factory internally.
+ storage_alice = get_or_new_user_skill_storage("alice", app_config=rich_config)
+ storage_bob = get_or_new_user_skill_storage("bob", app_config=rich_config)
+
+ # 1. Two users each create a custom skill named "report-gen"
+ storage_alice.write_custom_skill("report-gen", "SKILL.md", _skill_content("report-gen", "Alice report generator"))
+ storage_bob.write_custom_skill("report-gen", "SKILL.md", _skill_content("report-gen", "Bob report generator"))
+
+ # 2. Alice disables her "report-gen"
+ storage_alice.set_skill_enabled_state("report-gen", False)
+
+ # 3. Bob's "report-gen" stays enabled in load_skills()
+ bob_skills = storage_bob.load_skills(enabled_only=False)
+ bob_report = [s for s in bob_skills if s.name == "report-gen" and s.category == SkillCategory.CUSTOM]
+ assert len(bob_report) == 1
+ assert bob_report[0].enabled is True
+
+ # Complementary: Alice's "report-gen" is disabled
+ alice_skills = storage_alice.load_skills(enabled_only=False)
+ alice_report = [s for s in alice_skills if s.name == "report-gen" and s.category == SkillCategory.CUSTOM]
+ assert len(alice_report) == 1
+ assert alice_report[0].enabled is False
+
+ # enabled_only=True filtering is also isolated
+ bob_enabled = storage_bob.load_skills(enabled_only=True)
+ assert any(s.name == "report-gen" for s in bob_enabled)
+
+ alice_enabled = storage_alice.load_skills(enabled_only=True)
+ assert not any(s.name == "report-gen" for s in alice_enabled)
+
+ # 4. Bob's skill still appears in the prompt section
+ clear_skills_system_prompt_cache()
+ prompt = get_skills_prompt_section(user_id="bob", app_config=rich_config)
+ assert "report-gen" in prompt
+
+ # 5. _is_disabled_skill_path returns False for Bob's skill path
+ assert _is_disabled_skill_path("/mnt/skills/custom/report-gen/SKILL.md", user_id="bob") is False
+
+ # Complementary: Alice's skill path IS disabled
+ assert _is_disabled_skill_path("/mnt/skills/custom/report-gen/SKILL.md", user_id="alice") is True
+
+
+class TestSkillStateAtomicWrite:
+ """P2-2: ``_write_skill_states`` must be atomic so a crash mid-write
+ cannot silently re-enable every skill the user had disabled.
+ """
+
+ def test_writes_via_tempfile_then_replace(self, user_storage: UserScopedSkillStorage, base_dir: Path) -> None:
+ states = {"report-gen": {"enabled": False}}
+ user_storage._write_skill_states(states)
+
+ target = user_storage._skill_states_file
+ assert target.exists()
+ # No leftover .tmp files in the directory.
+ leftovers = [p for p in base_dir.glob("users/*/skills/.skill_states_*.json.tmp") if p.exists()]
+ assert not leftovers, f"temp file left behind: {leftovers}"
+ import json as _json
+
+ assert _json.loads(target.read_text(encoding="utf-8")) == states
+
+ def test_failed_write_does_not_truncate_existing_file(self, user_storage: UserScopedSkillStorage) -> None:
+ import json as _json
+
+ # Seed a valid state file.
+ target = user_storage._skill_states_file
+ target.parent.mkdir(parents=True, exist_ok=True)
+ target.write_text(_json.dumps({"old-skill": {"enabled": False}}), encoding="utf-8")
+
+ # Force the inner write to fail; ensure the existing file is intact.
+ with patch("pathlib.Path.replace", side_effect=OSError("boom")):
+ with pytest.raises(OSError):
+ user_storage._write_skill_states({"new-skill": {"enabled": True}})
+
+ # Pre-existing content must survive the failed replacement.
+ assert target.exists()
+ assert _json.loads(target.read_text(encoding="utf-8")) == {"old-skill": {"enabled": False}}
+
+ # And no orphan temp file should be left around in the user skills dir.
+ leftovers = list(target.parent.glob(".skill_states_*.json.tmp"))
+ assert not leftovers, f"temp file leaked: {leftovers}"
+
+
+class TestSkillStateFailClosed:
+ """P1-2: ``_is_disabled_skill_path`` must fail CLOSED (return True)
+ when the enabled state cannot be determined, so a corrupt
+ ``_skill_states.json`` or mid-write race never lets the agent read a
+ disabled skill's files.
+ """
+
+ def test_returns_true_when_state_lookup_raises(self) -> None:
+ from deerflow.sandbox.tools import _is_disabled_skill_path
+
+ def _boom(_skill_name: str) -> bool:
+ raise OSError("storage unavailable")
+
+ with patch("deerflow.skills.storage.user_scoped_skill_storage.UserScopedSkillStorage.get_skill_enabled_state", side_effect=_boom):
+ assert _is_disabled_skill_path("/mnt/skills/custom/report-gen/SKILL.md", user_id="default") is True
+
+ def test_returns_true_when_public_extensions_config_raises(self) -> None:
+ from deerflow.sandbox.tools import _is_disabled_skill_path
+
+ def _boom() -> bool:
+ raise OSError("extensions_config.json unreadable")
+
+ with patch("deerflow.config.extensions_config.ExtensionsConfig.from_file", side_effect=_boom):
+ assert _is_disabled_skill_path("/mnt/skills/public/bootstrap/SKILL.md", user_id="default") is True
+
+
+class TestSkillLoadingRespectsGlobalDisable:
+ """P2-1: when the global ``extensions_config.json`` disables a
+ CUSTOM/LEGACY skill, ``load_skills`` must still report it as
+ disabled even if the per-user state has no entry (defaulting to
+ enabled otherwise). Without the AND, an admin's global "off" for a
+ shared skill would be silently flipped to "on" the moment a new
+ user touches the per-user storage.
+ """
+
+ def test_global_disable_wins_when_per_user_state_missing(self, tmp_path: Path) -> None:
+ from types import SimpleNamespace
+
+ from deerflow.config.paths import Paths
+ from deerflow.skills.storage import get_or_new_user_skill_storage
+ from deerflow.skills.types import SkillCategory
+
+ base = tmp_path
+ skills_root = base / "skills"
+ skills_root.mkdir()
+ (skills_root / "custom").mkdir()
+ (skills_root / "custom" / "shared-skill").mkdir()
+ (skills_root / "custom" / "shared-skill" / "SKILL.md").write_text(
+ _skill_content("shared-skill"),
+ encoding="utf-8",
+ )
+
+ # Per-user state is empty (no per-user override for "shared-skill").
+ with patch("deerflow.config.paths.get_paths", return_value=Paths(base_dir=base)):
+ with patch("deerflow.config.paths._paths", None):
+ cfg = SimpleNamespace(
+ skills=SimpleNamespace(
+ get_skills_path=lambda: skills_root,
+ container_path="/mnt/skills",
+ use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
+ ),
+ )
+ with patch("deerflow.config.get_app_config", return_value=cfg):
+ # Global extensions_config reports the shared skill as disabled.
+ ext_cfg = SimpleNamespace(
+ skills={"shared-skill": SimpleNamespace(enabled=False)},
+ is_skill_enabled=lambda name, _cat: not (name == "shared-skill"),
+ )
+ # The function inside ``load_skills`` does a
+ # function-local ``from deerflow.config.extensions_config
+ # import get_extensions_config``, so patch the
+ # extension_config module symbol.
+ with patch("deerflow.config.extensions_config.get_extensions_config", return_value=ext_cfg):
+ storage = get_or_new_user_skill_storage("alice", app_config=cfg)
+ loaded = storage.load_skills(enabled_only=False)
+ shared = [s for s in loaded if s.name == "shared-skill" and s.category == SkillCategory.LEGACY]
+ assert len(shared) == 1
+ # AND-merge: per-user default True AND global False → False
+ assert shared[0].enabled is False
+
+
+class TestEnabledSkillsByConfigCacheBounded:
+ """P2-4: ``_enabled_skills_by_config_cache`` must be bounded so a
+ long-running process cannot leak one entry per distinct
+ (app_config, user_id) pair ever seen.
+ """
+
+ def test_evicts_least_recently_used_above_maxsize(self, monkeypatch) -> None:
+ from collections import OrderedDict
+
+ from deerflow.agents.lead_agent import prompt as prompt_module
+
+ # Shrink the cap so the test stays fast.
+ monkeypatch.setattr(prompt_module, "_ENABLED_SKILLS_BY_CONFIG_CACHE_MAXSIZE", 4)
+ prompt_module._enabled_skills_by_config_cache = OrderedDict()
+
+ class FakeConfig:
+ def __init__(self, name: str) -> None:
+ self.name = name
+
+ class FakeStorage:
+ def __init__(self) -> None:
+ self.load_calls = 0
+
+ def load_skills(self, *, enabled_only: bool = False):
+ self.load_calls += 1
+ return []
+
+ configs = [FakeConfig(f"cfg-{i}") for i in range(6)]
+ storages = [FakeStorage() for _ in range(6)]
+ # Index by cfg id so the lookups below are deterministic.
+ cfg_to_storage = {id(c): s for c, s in zip(configs, storages)}
+
+ def _user_storage(user_id, *, app_config=None):
+ return cfg_to_storage[id(app_config)]
+
+ def _global_storage(*, app_config=None):
+ return cfg_to_storage[id(app_config)]
+
+ # Patch the *already-imported* references inside the prompt
+ # module. ``from ... import`` binds the name at import time, so
+ # patching the storage module has no effect.
+ with patch.object(prompt_module, "get_or_new_user_skill_storage", side_effect=_user_storage), patch.object(prompt_module, "get_or_new_skill_storage", side_effect=_global_storage):
+ for i, cfg in enumerate(configs):
+ prompt_module.get_enabled_skills_for_config(app_config=cfg, user_id=f"user-{i}")
+
+ # After 6 distinct (cfg, user) inserts with a cap of 4, the cache
+ # must hold exactly the 4 most-recently-touched entries.
+ assert len(prompt_module._enabled_skills_by_config_cache) == 4
+ kept_keys = set(prompt_module._enabled_skills_by_config_cache.keys())
+ # The two oldest (cfg-0/user-0, cfg-1/user-1) should have been evicted.
+ assert (id(configs[0]), "user-0") not in kept_keys
+ assert (id(configs[1]), "user-1") not in kept_keys
+ # And the four newest must still be there.
+ for i in range(2, 6):
+ assert (id(configs[i]), f"user-{i}") in kept_keys
+
+ # Touching an older key bumps it to MRU. ``configs[0]`` is no
+ # longer in the cache, so this re-loads it; this also re-loads
+ # ``storages[0]`` from disk (so its load_calls goes from 0 to 1).
+ with patch.object(prompt_module, "get_or_new_user_skill_storage", side_effect=_user_storage), patch.object(prompt_module, "get_or_new_skill_storage", side_effect=_global_storage):
+ prompt_module.get_enabled_skills_for_config(app_config=configs[0], user_id="user-0")
+ # Now configs[0] is MRU. Insert a new (cfg-new, user-new) entry:
+ # the LRU is configs[2] and must be evicted.
+ new_cfg = FakeConfig("cfg-new")
+ new_storage = FakeStorage()
+ cfg_to_storage[id(new_cfg)] = new_storage
+ with patch.object(prompt_module, "get_or_new_user_skill_storage", side_effect=_user_storage), patch.object(prompt_module, "get_or_new_skill_storage", side_effect=_global_storage):
+ prompt_module.get_enabled_skills_for_config(app_config=new_cfg, user_id="user-new")
+
+ kept = set(prompt_module._enabled_skills_by_config_cache.keys())
+ assert (id(configs[0]), "user-0") in kept, "MRU touch should keep configs[0]"
+ assert (id(new_cfg), "user-new") in kept
+ assert (id(configs[2]), "user-2") not in kept, "LRU should have been evicted"
+ assert len(prompt_module._enabled_skills_by_config_cache) == 4
diff --git a/frontend/src/core/skills/type.ts b/frontend/src/core/skills/type.ts
index 7bb76a215..1d879be6c 100644
--- a/frontend/src/core/skills/type.ts
+++ b/frontend/src/core/skills/type.ts
@@ -4,4 +4,5 @@ export interface Skill {
category: string;
license: string;
enabled: boolean;
+ editable: boolean;
}
diff --git a/skills/public/skill-creator/SKILL.md b/skills/public/skill-creator/SKILL.md
index 65b3a402d..99e953b55 100644
--- a/skills/public/skill-creator/SKILL.md
+++ b/skills/public/skill-creator/SKILL.md
@@ -29,6 +29,55 @@ Then after the skill is done (but again, the order is flexible), you can also ru
Cool? Cool.
+---
+
+## DeerFlow Environment (⚠️ READ THIS FIRST)
+
+If you are running inside a **DeerFlow** sandboxed agent environment (you have access to the `skill_manage` tool), you MUST follow these rules for all skill file operations. These override the generic file-writing and packaging instructions below.
+
+### Why this matters
+
+In DeerFlow, the sandbox filesystem is isolated. Files written with `write_file` land in `/mnt/user-data/outputs/`, which is a **per-thread output directory** — new chats cannot see files there. Skills must be persisted through the dedicated `skill_manage` tool so they are stored in the per-user skill directory and immediately visible to all future chats.
+
+### Use `skill_manage` for all skill file operations
+
+| Operation | skill_manage action | Example |
+|-----------|---------------------|---------|
+| Create a new skill | `action="create"` | `skill_manage(action="create", name="my-skill", content="---\nname: my-skill\n---\n...")` |
+| Replace entire SKILL.md | `action="edit"` | `skill_manage(action="edit", name="my-skill", content="updated SKILL.md")` |
+| Partial edit (find & replace) | `action="patch"` | `skill_manage(action="patch", name="my-skill", find="old text", replace="new text")` |
+| Delete a skill | `action="delete"` | `skill_manage(action="delete", name="my-skill")` |
+| Add a supporting file | `action="write_file"` | `skill_manage(action="write_file", name="my-skill", path="scripts/helper.py", content="...")` |
+| Remove a supporting file | `action="remove_file"` | `skill_manage(action="remove_file", name="my-skill", path="scripts/helper.py")` |
+
+### Key rules
+
+1. **NEVER use sandbox `write_file` to create or modify skill files** (SKILL.md, scripts/, references/, assets/). These would land in `/mnt/user-data/outputs/` and be invisible to future chats. Always use `skill_manage` instead.
+
+2. **Skip the `package_skill.py` step**. In DeerFlow, `skill_manage` already persists the skill to the correct per-user directory. No `.skill` packaging or manual install is needed. The skill is immediately available in all new chats.
+
+3. **Skip the `present_files` step for skills**. Skills are NOT deliverables — they are persisted via `skill_manage` and auto-loaded by the skill system. Only use `present_files` for non-skill outputs (eval reports, benchmarks, etc.).
+
+4. **Eval workspace files are OK in sandbox**. Test prompts, benchmark data, eval viewer HTML, grading results, etc. are NOT skill files — you can write these to `/mnt/user-data/outputs/` or `/mnt/user-data/workspace/` using sandbox `write_file` as usual.
+
+5. **To read an existing skill's SKILL.md**, use `read_file("/mnt/skills/custom//SKILL.md")` in the sandbox (it maps to the per-user skill directory).
+
+6. **Updating an existing skill**: use `skill_manage(action="edit")` or `skill_manage(action="patch")`. Do NOT copy to `/tmp/` first — `skill_manage` handles the per-user storage directly.
+
+### Workflow in DeerFlow
+
+The core loop is the same, but the persistence mechanism changes:
+
+1. Capture intent → interview → draft SKILL.md content
+2. **Call `skill_manage(action="create", name=, content=)`** to persist the skill
+3. Run test cases (eval workspace files use sandbox `write_file`)
+4. Evaluate results, gather feedback
+5. **Call `skill_manage(action="edit")` or `skill_manage(action="patch")`** to improve the skill
+6. Repeat until satisfied
+7. **Done — no packaging needed.** The skill is already persisted and visible to all future chats.
+
+---
+
## Communicating with the user
The skill creator is liable to be used by people across a wide range of familiarity with coding jargon. If you haven't heard (and how could you, it's only very recently that it started), there's a trend now where the power of Claude is inspiring plumbers to open up their terminals, parents and grandparents to google "how to install npm". On the other hand, the bulk of users are probably fairly computer-literate.