fix(skills): unlink a symlinked custom skill package on delete (#5881)

* fix(skills): unlink a symlinked custom skill package on delete

The storage contract (_is_external_skill_directory_symlink) accepts a
one-level symlink under custom/ that points at an operator-managed skill
package, and every read path honours it: the skill loads, lists, validates
and can be enabled. delete_custom_skill, however, removed the package with
shutil.rmtree, which refuses symlinks. The delete raised OSError after the
history record had already been appended, skill_projection_mutation cleared
the user's projected custom view on the way out (rebuilt on the next
acquire), and DELETE /api/skills/custom/{name} answered 500 — for both
LocalSkillStorage and UserScopedSkillStorage, which inherits the method.

Unlink the entry when it is a symlink and keep rmtree for real directories.
The external tree is not ours to delete: removing the skill means removing
the link. Checking is_symlink() first also covers a dangling link, which
exists() would report as absent.

Tests cover both storages: the link is gone, the skill no longer exists,
the external package (SKILL.md and a support file) is untouched, the
history record was written, and the sibling skill's projected view
survives the delete.

* docs(changelog): reference #5881 in the symlinked-skill delete entries

CHANGELOG.md

@@ -323,6 +323,14 @@ This release closes that milestone with **181 merged pull requests**.
 
 ### Fixed
 
+- **skills:** Deleting a custom skill whose package directory is an
+  operator-managed symlink now succeeds. The storage contract accepts a
+  one-level link under `custom/`, but `delete_custom_skill` removed the
+  package with `shutil.rmtree`, which refuses symlinks: the delete failed with
+  `OSError` after its history record had been written, the user's projected
+  skill view was cleared on the way out, and `DELETE /api/skills/custom/{name}`
+  answered 500. The link itself is now removed; the external tree it points
+  at is never touched. ([#5881])
 - **frontend:** Stop mutating subtask render state during `MessageList` render.
   Subtask synchronization moved from render into an effect, so cards immediately
   receive pure message-derived snapshots even before the task context publishes
@@ -6094,4 +6102,5 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
 [#5855]: https://github.com/bytedance/deer-flow/pull/5855
 [#5856]: https://github.com/bytedance/deer-flow/pull/5856
 [#5859]: https://github.com/bytedance/deer-flow/pull/5859
+[#5881]: https://github.com/bytedance/deer-flow/pull/5881
 

CHANGELOG_zh.md

@@ -328,6 +328,10 @@
 
 ### 修复
 
+- **技能：** 删除包目录为运维方管理的符号链接的自定义技能现在可以成功。存储契约允许
+  `custom/` 下的一级链接，但 `delete_custom_skill` 用 `shutil.rmtree` 删除包目录，而它拒绝
+  符号链接：删除在写入历史记录之后以 `OSError` 失败，退出时清空了用户的技能投影视图，
+  `DELETE /api/skills/custom/{name}` 返回 500。现在只移除链接本身，绝不触碰其指向的外部目录。([#5881])
 - **前端：** 子任务渲染状态不再在 `MessageList` 渲染过程中被就地修
   改。子任务同步从渲染阶段移入 effect，因此即使任务上下文尚未发布
   更新，卡片也能立即拿到纯派生自消息的快照——修复了最终流式参数与
@@ -5188,3 +5192,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子
 [#5855]: https://github.com/bytedance/deer-flow/pull/5855
 [#5856]: https://github.com/bytedance/deer-flow/pull/5856
 [#5859]: https://github.com/bytedance/deer-flow/pull/5859
+[#5881]: https://github.com/bytedance/deer-flow/pull/5881

backend/packages/harness/deerflow/skills/AGENTS.md

@@ -2,7 +2,7 @@
 
 - **Location**: global public skills live under `deer-flow/skills/public/`; user-authored custom skills live under `{DEER_FLOW_HOME}/users/{user_id}/skills/custom/`; globally managed integration skills live under `{DEER_FLOW_HOME}/integrations/skills/{provider}/`; per-user integration credentials remain under `{DEER_FLOW_HOME}/users/{user_id}/integrations/{provider}/{config,data}`
 - **Format**: Directory with `SKILL.md` (YAML frontmatter: name, description, license, allowed-tools as a spec-compatible string or YAML list, argument-hint, required-secrets). Exact portable spellings such as `Bash`, `WebFetch`, `WebSearch`, `Glob`, `Grep`, `Read`, `Write`, and `Edit` map to `bash`, `web_fetch`, `web_search`, `glob`, `grep`, `read_file`, `write_file`, and `str_replace`; lowercase or otherwise unknown scalar names and YAML-list entries preserve their exact runtime spelling. Argument-scoped entries remain literal and inactive because the tool policy does not inspect arguments; the scalar tokenizer keeps spaces, quotes, and escaped parentheses inside patterns intact.
-- **Loading**: `load_skills()` recursively scans public, per-user custom, global integration, and legacy custom locations for `SKILL.md`, parses metadata, and reads enabled state from extensions_config.json plus per-user skill state for non-public categories; that directory is a package boundary, so no nested `SKILL.md` is registered as a runtime skill. A custom skill directory may be a one-level symlink to an external directory for compatibility with operator-managed skill trees; activation still rejects a symlinked `SKILL.md` or deeper path escape. SkillScan has a deliberately narrower packaging rule: known eval fixtures are permitted as support data, while other nested `SKILL.md` files are reported as package defects. It parses runtime metadata and reads enabled state from extensions_config.json.
+- **Loading**: `load_skills()` recursively scans public, per-user custom, global integration, and legacy custom locations for `SKILL.md`, parses metadata, and reads enabled state from extensions_config.json plus per-user skill state for non-public categories; that directory is a package boundary, so no nested `SKILL.md` is registered as a runtime skill. A custom skill directory may be a one-level symlink to an external directory for compatibility with operator-managed skill trees; activation still rejects a symlinked `SKILL.md` or deeper path escape. Deleting such a skill unlinks the entry and leaves the external tree untouched. SkillScan has a deliberately narrower packaging rule: known eval fixtures are permitted as support data, while other nested `SKILL.md` files are reported as package defects. It parses runtime metadata and reads enabled state from extensions_config.json.
 - **External reload**: `POST /api/skills/reload` is an admin-only, process-local invalidation hook for trusted MinIO/NFS/CSI writes. `SkillStorage` instances do not cache a catalog — `load_skills()` scans on every call — so the route clears all `(app_config, user_id)` entries and the rendered prompt-section LRU, then waits up to the shared refresh timeout for the existing off-loop single-flight refresh. Each invalidation receives a generation-bound result handle; a successful scan atomically replaces the global enabled-skills cache, while a loader-level failure propagates to the HTTP waiter and preserves the last-known-good global cache. Per-user/config scans capture the refresh version and cannot repopulate shared caches if invalidation occurs while they are loading. A timed-out HTTP wait fails generically while the daemon refresh worker continues. Subsequent runs rescan after a successful reload; active runs keep their existing snapshot. Each Uvicorn worker/Kubernetes Pod must be targeted separately. Direct mount writes bypass install/edit validation, SkillScan, and history, so mounted roots are an operator-controlled trust boundary.
 - **Tool policy**: Agent `allowed-tools` declarations apply dynamically only to slash-activated skills and skills captured in `ThreadState.skill_context` through configured `read_file` loads; passive enabled skills and skill allowlists do not clamp the baseline tool set. A lead custom Agent's explicit `skills` list is additionally enforced at the sandbox filesystem layer (see Sandbox projection); subagent skill lists still scope discovery and activation only because concurrently delegated subagents share the lead thread sandbox. Subagents render only skill discovery metadata at startup and reuse the same adjacent `SkillActivationMiddleware` + `SkillToolPolicyMiddleware` pair as the lead; their configured `skills` field limits discovery and activation instead of eagerly loading bodies or unioning policies. Slash policy is dominant for its run, preventing subsequently read skills from widening explicit authority; autonomous captured skills use the existing union only when no slash source exists. `tool_search` and `describe_skill` stay available as framework discovery infrastructure, while every discovered or promoted business tool still requires active-policy permission for schema visibility and execution; `task`, `list_background_tasks`, and `cancel_background_task` likewise require explicit declarations. Each active model call intentionally reloads the full live registry so enable/disable changes, frontmatter edits, and custom/public name-shadow winners take effect without a stale TTL or unsafe direct-path cache; all tool calls produced by that model step reuse the resulting source-and-path-signed decision. Registry failures and all-invalid active sets fail closed, while stale individual paths are skipped when another valid skill remains. The dynamic `allowed-tools` policy remains best-effort behavioral scoping: alternate loading paths are not captured and bounded autonomous context may evict entries.
 - **Sandbox projection**: `skills/projection.py` materializes enabled-only shared trees at `{base_dir}/skills_view/public` and `{base_dir}/users/{user_id}/skills_view/{custom,legacy,integrations}`. A lead Agent with an explicit `skills` allowlist (including `[]`) gets the intersection of enabled public/user-visible skills and that allowlist at `{base_dir}/users/{user_id}/threads/{thread_id}/skills_view/{public,custom,legacy,integrations}`. `skills=None` keeps the shared zero-copy mount until a thread has used an explicit policy; later unrestricted runs repopulate the same stable thread root with all enabled skills. Rebuilds sign source state, view state, and normalized policy in a manifest, revoke every old category before adding the new policy, stage copies in temporary directories, and atomically replace files. Category root inodes stay stable for live bind mounts; concurrent readers can briefly see fewer skills during a policy change, never a skill revoked by the new policy. Policy-scoped copies reject absolute symlinks and relative symlinks that resolve outside their own skill package, preventing a permitted package from linking back to an omitted source. It copies files into the view (`_copy_into_view`) so a sandbox write cannot mutate the canonical skill inode; the operational trade-off is an O(total bytes) I/O and per-user/thread storage multiplier across rebuilds, prioritized for write isolation over zero-copy hardlinks. Steady-state freshness checks combine source and view metadata tree digests, so in-sandbox view tampering is detected and repaired on the next acquire. Storage writes, archive installs, deletes, and toggles rebuild shared scopes under a cross-process lock; Gateway boot ensures only the shared public view, user views are repaired lazily on acquire, and Agent thread views are recomputed before sandbox reuse. Managed integration packages are global, but their projected category is per-user because enabled state is isolated. User projection rebuilds re-read global enable state from disk instead of the process singleton, so a toggle handled by another Gateway worker is reflected on the next acquire. Gateway public-skill toggles take the public projection lock before the shared `extensions_config_write_lock`, re-read an existing config raw from disk, change only that skill's entry, and rebuild before responding; keep this as one worker-owned critical section so MCP writes cannot interleave and request cancellation cannot release either lock while the worker still runs. The shared public steady-state signature check runs without the global projection lock; stale/error paths take the lock and re-check before rebuilding or clearing. User and thread scope checks are serialized per scope. Projection failures clear the affected view before raising.

backend/packages/harness/deerflow/skills/storage/local_skill_storage.py

@@ -231,7 +231,13 @@ class LocalSkillStorage(SkillStorage):
                 )
         removal = ((SkillCategory.CUSTOM, Path(name)),)
         with self._skill_projection_mutation(remove=removal):
-            if target.exists():
+            if target.is_symlink():
+                # An operator-linked package (see
+                # ``_is_external_skill_directory_symlink``): the skill is the
+                # link, and the external tree is not ours to delete.
+                # ``shutil.rmtree`` refuses symlinks, so unlink instead.
+                target.unlink()
+            elif target.exists():
                 shutil.rmtree(target)
 
     def _skill_projection_mutation(

backend/tests/test_skill_storage_delete_symlinked_package.py

@@ -0,0 +1,101 @@
+"""Deleting an operator-linked custom skill package must unlink, not fail.
+
+The storage contract (``SkillStorage._is_external_skill_directory_symlink``)
+accepts a one-level symlink under ``custom/`` that points at an externally
+managed skill package. Every read path honors it, but ``delete_custom_skill``
+called ``shutil.rmtree`` on the link, which refuses symlinks: the delete
+raised ``OSError`` after the history record had already been appended, the
+projection mutation cleared the user's skill view on the way out, and the
+Gateway answered 500. The external tree is not ours to delete, so removing
+the skill means removing the link.
+"""
+
+from __future__ import annotations
+
+import json
+from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import patch
+
+import pytest
+from support.symlinks import symlink_or_skip
+
+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
+
+
+def _skill_content(name: str) -> str:
+    return f"---\nname: {name}\ndescription: Linked skill\n---\n\n# {name}\n"
+
+
+@pytest.fixture(autouse=True)
+def _reset_storages():
+    reset_skill_storage()
+    yield
+    reset_skill_storage()
+
+
+def _plant_external_package(tmp_path: Path, name: str) -> Path:
+    external = tmp_path / "operator-skills" / name
+    external.mkdir(parents=True)
+    (external / "SKILL.md").write_text(_skill_content(name), encoding="utf-8")
+    (external / "references").mkdir()
+    (external / "references" / "notes.md").write_text("keep me", encoding="utf-8")
+    return external
+
+
+def _assert_external_untouched(external: Path, name: str) -> None:
+    assert (external / "SKILL.md").read_text(encoding="utf-8") == _skill_content(name)
+    assert (external / "references" / "notes.md").read_text(encoding="utf-8") == "keep me"
+
+
+def test_local_storage_unlinks_a_symlinked_package_and_leaves_the_target_alone(tmp_path: Path):
+    skills_root = tmp_path / "skills"
+    (skills_root / "custom").mkdir(parents=True)
+    storage = get_or_new_skill_storage(skills_path=str(skills_root))
+    external = _plant_external_package(tmp_path, "linked-skill")
+    link = skills_root / "custom" / "linked-skill"
+    symlink_or_skip(link, external, target_is_directory=True)
+    assert storage.custom_skill_exists("linked-skill")
+
+    storage.delete_custom_skill("linked-skill", history_meta={"action": "delete", "source": "test"})
+
+    assert not link.exists() and not link.is_symlink()
+    assert not storage.custom_skill_exists("linked-skill")
+    _assert_external_untouched(external, "linked-skill")
+    history = [json.loads(line) for line in storage.get_skill_history_file("linked-skill").read_text(encoding="utf-8").splitlines() if line]
+    assert history[-1]["action"] == "delete"
+    assert history[-1]["prev_content"] == _skill_content("linked-skill")
+
+
+def test_user_scoped_storage_unlinks_a_symlinked_package_and_keeps_the_projection(tmp_path: Path):
+    skills_root = tmp_path / "skills"
+    (skills_root / "public").mkdir(parents=True)
+    (skills_root / "custom").mkdir()
+    config = 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.paths.get_paths", return_value=Paths(base_dir=tmp_path)), patch("deerflow.config.paths._paths", None):
+        storage = UserScopedSkillStorage("test-user", host_path=str(skills_root), app_config=config)
+        storage.write_custom_skill("kept-skill", "SKILL.md", _skill_content("kept-skill"))
+        external = _plant_external_package(tmp_path, "linked-skill")
+        link = storage.get_user_custom_root() / "linked-skill"
+        symlink_or_skip(link, external, target_is_directory=True)
+        assert storage.custom_skill_exists("linked-skill")
+
+        storage.delete_custom_skill("linked-skill")
+
+        assert not link.exists() and not link.is_symlink()
+        assert not storage.custom_skill_exists("linked-skill")
+        _assert_external_untouched(external, "linked-skill")
+        # The sibling skill is still there and still projected: a failed
+        # mutation would have cleared the user's whole custom view.
+        assert storage.custom_skill_exists("kept-skill")
+        view = Paths(base_dir=tmp_path).base_dir / "users" / "test-user" / "skills_view" / "custom"
+        assert (view / "kept-skill" / "SKILL.md").is_file()
+        assert not (view / "linked-skill").exists()