fix(skill): 修复 skills 在文件系统的可访问性的错误并优化并发体验
This commit is contained in:
Wenjie Zhang
parent
0836fd8d4f
commit
cea53cc67c
@ -69,7 +69,9 @@ class CustomCompositeBackend(CompositeBackend):
|
||||
def _get_visible_skills_from_runtime(runtime) -> list[str]:
|
||||
"""获取运行时可见的 skills 列表"""
|
||||
context = getattr(runtime, "context", None)
|
||||
selected = getattr(context, "skills", None) or []
|
||||
selected = getattr(context, "_visible_skills", None)
|
||||
if not isinstance(selected, list):
|
||||
selected = getattr(context, "skills", None) or []
|
||||
return normalize_selected_skills(selected)
|
||||
|
||||
|
||||
@ -94,7 +96,7 @@ def create_agent_composite_backend(runtime) -> CompositeBackend:
|
||||
visible_skills = _get_visible_skills_from_runtime(runtime)
|
||||
thread_id = _extract_thread_id(runtime)
|
||||
return CustomCompositeBackend(
|
||||
default=ProvisionerSandboxBackend(thread_id=thread_id),
|
||||
default=ProvisionerSandboxBackend(thread_id=thread_id, visible_skills=visible_skills),
|
||||
routes={
|
||||
"/skills/": SelectedSkillsReadonlyBackend(selected_slugs=visible_skills),
|
||||
},
|
||||
|
||||
@ -17,6 +17,7 @@ from deepagents.backends.protocol import (
|
||||
from deepagents.backends.sandbox import BaseSandbox
|
||||
|
||||
from yuxi import config as conf
|
||||
from yuxi.services.skill_service import sync_thread_visible_skills
|
||||
from yuxi.utils.logging_config import logger
|
||||
|
||||
from .provider import get_sandbox_provider, sandbox_id_for_thread
|
||||
@ -61,11 +62,12 @@ def _looks_like_binary(content: bytes) -> bool:
|
||||
|
||||
|
||||
class ProvisionerSandboxBackend(BaseSandbox):
|
||||
def __init__(self, thread_id: str):
|
||||
def __init__(self, thread_id: str, *, visible_skills: list[str] | None = None):
|
||||
self._thread_id = str(thread_id or "").strip()
|
||||
if not self._thread_id:
|
||||
raise ValueError("thread_id is required for ProvisionerSandboxBackend")
|
||||
|
||||
self._visible_skills = list(visible_skills or [])
|
||||
self._provider = get_sandbox_provider()
|
||||
self._id = sandbox_id_for_thread(self._thread_id)
|
||||
self._client: Any | None = None
|
||||
@ -88,6 +90,7 @@ class ProvisionerSandboxBackend(BaseSandbox):
|
||||
return AgentSandboxClient(base_url=sandbox_url, timeout=self._command_timeout_seconds)
|
||||
|
||||
def _get_client(self) -> Any:
|
||||
sync_thread_visible_skills(self._thread_id, self._visible_skills)
|
||||
connection = self._provider.get(self._thread_id, create_if_missing=True)
|
||||
if connection is None:
|
||||
raise RuntimeError(f"sandbox is unavailable for thread {self._thread_id}")
|
||||
|
||||
@ -4,6 +4,7 @@ import asyncio
|
||||
import re
|
||||
import shutil
|
||||
import tempfile
|
||||
import threading
|
||||
import uuid
|
||||
import zipfile
|
||||
from pathlib import Path, PurePosixPath
|
||||
@ -52,6 +53,17 @@ TEXT_FILE_EXTENSIONS = {
|
||||
}
|
||||
|
||||
BUILTIN_SKILL_OPERATOR = "builtin-system"
|
||||
_THREAD_SKILLS_LOCK = threading.Lock()
|
||||
_THREAD_SKILLS_LOCKS: dict[str, threading.Lock] = {}
|
||||
|
||||
|
||||
def _get_thread_skills_lock(thread_id: str) -> threading.Lock:
|
||||
with _THREAD_SKILLS_LOCK:
|
||||
lock = _THREAD_SKILLS_LOCKS.get(thread_id)
|
||||
if lock is None:
|
||||
lock = threading.Lock()
|
||||
_THREAD_SKILLS_LOCKS[thread_id] = lock
|
||||
return lock
|
||||
|
||||
|
||||
def _normalize_string_list(values: list[str] | None) -> list[str]:
|
||||
@ -82,6 +94,69 @@ def get_skills_root_dir() -> Path:
|
||||
return root
|
||||
|
||||
|
||||
def get_thread_skills_root_dir(thread_id: str) -> Path:
|
||||
safe_thread_id = str(thread_id or "").strip()
|
||||
if not safe_thread_id:
|
||||
raise ValueError("thread_id is required")
|
||||
if not re.fullmatch(r"[A-Za-z0-9_-]+", safe_thread_id):
|
||||
raise ValueError("thread_id contains invalid characters")
|
||||
|
||||
root = Path(sys_config.save_dir) / "threads" / safe_thread_id / "skills"
|
||||
root.mkdir(parents=True, exist_ok=True)
|
||||
return root
|
||||
|
||||
|
||||
def sync_thread_visible_skills(thread_id: str, selected_slugs: list[str] | None) -> Path:
|
||||
skills_root = get_skills_root_dir().resolve()
|
||||
thread_skills_root = get_thread_skills_root_dir(thread_id)
|
||||
normalized_slugs = [slug for slug in _normalize_string_list(selected_slugs) if is_valid_skill_slug(slug)]
|
||||
visible_slugs = set(normalized_slugs)
|
||||
with _get_thread_skills_lock(thread_id):
|
||||
for entry in thread_skills_root.iterdir():
|
||||
if entry.name in visible_slugs:
|
||||
continue
|
||||
if entry.is_dir() and not entry.is_symlink():
|
||||
shutil.rmtree(entry)
|
||||
else:
|
||||
entry.unlink()
|
||||
|
||||
for slug in normalized_slugs:
|
||||
source_dir = (skills_root / slug).resolve()
|
||||
target_dir = thread_skills_root / slug
|
||||
|
||||
try:
|
||||
source_dir.relative_to(skills_root)
|
||||
except ValueError:
|
||||
continue
|
||||
if not source_dir.is_dir():
|
||||
if target_dir.exists() or target_dir.is_symlink():
|
||||
if target_dir.is_dir() and not target_dir.is_symlink():
|
||||
shutil.rmtree(target_dir)
|
||||
else:
|
||||
target_dir.unlink()
|
||||
continue
|
||||
|
||||
if target_dir.exists():
|
||||
if target_dir.is_symlink():
|
||||
target_dir.unlink()
|
||||
elif target_dir.is_dir():
|
||||
if _dirs_equal(target_dir, source_dir):
|
||||
continue
|
||||
shutil.rmtree(target_dir)
|
||||
else:
|
||||
target_dir.unlink()
|
||||
|
||||
temp_target = thread_skills_root / f".{slug}.tmp-{uuid.uuid4().hex[:8]}"
|
||||
try:
|
||||
shutil.copytree(source_dir, temp_target, symlinks=False)
|
||||
temp_target.rename(target_dir)
|
||||
finally:
|
||||
if temp_target.exists():
|
||||
shutil.rmtree(temp_target, ignore_errors=True)
|
||||
|
||||
return thread_skills_root
|
||||
|
||||
|
||||
def get_builtin_skill_specs() -> list[Any]:
|
||||
from yuxi.agents.skills.buildin import BUILTIN_SKILLS
|
||||
|
||||
|
||||
@ -26,6 +26,7 @@ def test_create_agent_composite_backend_uses_provisioner_default(monkeypatch):
|
||||
backend = create_agent_composite_backend(_runtime(skills=["reporter"]))
|
||||
|
||||
assert isinstance(backend.default, ProvisionerSandboxBackend)
|
||||
assert backend.default._visible_skills == ["reporter"]
|
||||
assert "/skills/" in backend.routes
|
||||
|
||||
|
||||
|
||||
@ -43,6 +43,27 @@ def test_is_valid_skill_slug():
|
||||
assert svc.is_valid_skill_slug("") is False
|
||||
|
||||
|
||||
def test_sync_thread_visible_skills_only_keeps_selected(tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
|
||||
monkeypatch.setattr(svc.sys_config, "save_dir", str(tmp_path))
|
||||
skills_root = tmp_path / "skills"
|
||||
(skills_root / "alpha").mkdir(parents=True, exist_ok=True)
|
||||
(skills_root / "alpha" / "SKILL.md").write_text("alpha", encoding="utf-8")
|
||||
(skills_root / "beta").mkdir(parents=True, exist_ok=True)
|
||||
(skills_root / "beta" / "SKILL.md").write_text("beta", encoding="utf-8")
|
||||
|
||||
thread_root = svc.sync_thread_visible_skills("thread_1", ["alpha", "missing", "alpha"])
|
||||
|
||||
assert thread_root == tmp_path / "threads" / "thread_1" / "skills"
|
||||
assert sorted(path.name for path in thread_root.iterdir()) == ["alpha"]
|
||||
assert (thread_root / "alpha").is_dir()
|
||||
assert not (thread_root / "alpha").is_symlink()
|
||||
assert (thread_root / "alpha" / "SKILL.md").read_text(encoding="utf-8") == "alpha"
|
||||
|
||||
svc.sync_thread_visible_skills("thread_1", ["beta"])
|
||||
assert sorted(path.name for path in thread_root.iterdir()) == ["beta"]
|
||||
assert (thread_root / "beta" / "SKILL.md").read_text(encoding="utf-8") == "beta"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_skill_dependency_options(monkeypatch: pytest.MonkeyPatch):
|
||||
# Mock get_tool_metadata to return tool list
|
||||
@ -290,11 +311,7 @@ async def test_init_builtin_skills_create_missing(tmp_path: Path, monkeypatch: p
|
||||
source_dir = tmp_path / "builtin-skills" / "reporter"
|
||||
source_dir.mkdir(parents=True, exist_ok=True)
|
||||
(source_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"name: reporter\n"
|
||||
"description: SQL report\n"
|
||||
"---\n"
|
||||
"# SQL Reporter\n",
|
||||
"---\nname: reporter\ndescription: SQL report\n---\n# SQL Reporter\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
(source_dir / "prompts").mkdir(parents=True, exist_ok=True)
|
||||
@ -387,11 +404,7 @@ async def test_init_builtin_skills_updates_existing_record(tmp_path: Path, monke
|
||||
source_dir = tmp_path / "builtin-skills" / "reporter"
|
||||
source_dir.mkdir(parents=True, exist_ok=True)
|
||||
(source_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"name: reporter\n"
|
||||
"description: old\n"
|
||||
"---\n"
|
||||
"# SQL Reporter\n",
|
||||
"---\nname: reporter\ndescription: old\n---\n# SQL Reporter\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
@ -118,7 +118,6 @@ class LocalContainerProvisionerBackend:
|
||||
)
|
||||
self._network = os.getenv("DOCKER_NETWORK")
|
||||
self._threads_host_path = os.getenv("DOCKER_THREADS_HOST_PATH")
|
||||
self._skills_host_path = os.getenv("DOCKER_SKILLS_HOST_PATH")
|
||||
self._container_prefix = os.getenv("DOCKER_SANDBOX_PREFIX", "yuxi-sandbox")
|
||||
self._sandbox_host = os.getenv("DOCKER_SANDBOX_HOST", "host.docker.internal")
|
||||
self._health_timeout_seconds = int(os.getenv("SANDBOX_HEALTH_TIMEOUT_SECONDS", "300"))
|
||||
@ -131,7 +130,6 @@ class LocalContainerProvisionerBackend:
|
||||
|
||||
self._resolve_host_paths()
|
||||
self._threads_host_path = self._normalize_host_bind_path(self._threads_host_path)
|
||||
self._skills_host_path = self._normalize_host_bind_path(self._skills_host_path)
|
||||
|
||||
@staticmethod
|
||||
def _normalize_host_bind_path(path_value: str | None) -> str:
|
||||
@ -170,8 +168,27 @@ class LocalContainerProvisionerBackend:
|
||||
def _container_name(self, sandbox_id: str) -> str:
|
||||
return f"{self._container_prefix}-{self._sanitize_id(sandbox_id)}"
|
||||
|
||||
def _thread_skills_host_path(self, thread_id: str) -> Path:
|
||||
threads_root = Path(self._threads_host_path).resolve()
|
||||
thread_skills = (threads_root / thread_id / "skills").resolve()
|
||||
try:
|
||||
thread_skills.relative_to(threads_root)
|
||||
except ValueError as exc:
|
||||
raise ValueError("thread skills path resolved outside threads host root") from exc
|
||||
return thread_skills
|
||||
|
||||
def _is_expected_skills_mount(self, container, thread_id: str) -> bool:
|
||||
expected_source = str(self._thread_skills_host_path(thread_id))
|
||||
for mount in container.attrs.get("Mounts") or []:
|
||||
destination = (mount.get("Destination") or "").rstrip("/")
|
||||
if destination != "/home/yuxi/skills":
|
||||
continue
|
||||
source = str(mount.get("Source") or "").rstrip("/")
|
||||
return source == expected_source
|
||||
return False
|
||||
|
||||
def _resolve_host_paths(self) -> None:
|
||||
if self._threads_host_path and self._skills_host_path:
|
||||
if self._threads_host_path:
|
||||
return
|
||||
|
||||
container_id = os.getenv("HOSTNAME", "").strip()
|
||||
@ -194,9 +211,6 @@ class LocalContainerProvisionerBackend:
|
||||
base = Path(self._normalize_host_bind_path(saves_source))
|
||||
if not self._threads_host_path:
|
||||
self._threads_host_path = str(base / "threads")
|
||||
if not self._skills_host_path:
|
||||
self._skills_host_path = str(base / "skills")
|
||||
|
||||
def _host_port_for(self, container) -> int | None:
|
||||
ports = (container.attrs.get("NetworkSettings") or {}).get("Ports") or {}
|
||||
bindings = ports.get(f"{self._container_port}/tcp")
|
||||
@ -252,6 +266,11 @@ class LocalContainerProvisionerBackend:
|
||||
existing = self._get_container(sandbox_id)
|
||||
if existing is not None:
|
||||
existing.reload()
|
||||
if not self._is_expected_skills_mount(existing, safe_thread_id):
|
||||
logger.info("Recreating sandbox %s because skills mount is stale", sandbox_id)
|
||||
self.delete(sandbox_id)
|
||||
existing = None
|
||||
if existing is not None:
|
||||
if existing.status == "running":
|
||||
try:
|
||||
self._ensure_user_data_writable(existing)
|
||||
@ -277,8 +296,8 @@ class LocalContainerProvisionerBackend:
|
||||
raise ValueError("thread_id resolved outside threads host root") from exc
|
||||
thread_user_data.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
skills_path = Path(self._skills_host_path)
|
||||
skills_path.mkdir(parents=True, exist_ok=True)
|
||||
thread_skills = self._thread_skills_host_path(safe_thread_id)
|
||||
thread_skills.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
container_name = self._container_name(sandbox_id)
|
||||
run_kwargs = {
|
||||
@ -292,7 +311,7 @@ class LocalContainerProvisionerBackend:
|
||||
},
|
||||
"volumes": {
|
||||
str(thread_user_data): {"bind": "/home/yuxi/user-data", "mode": "rw"},
|
||||
str(skills_path): {"bind": "/home/yuxi/skills", "mode": "ro"},
|
||||
str(thread_skills): {"bind": "/home/yuxi/skills", "mode": "ro"},
|
||||
},
|
||||
"ports": {f"{self._container_port}/tcp": None},
|
||||
"security_opt": ["seccomp=unconfined"],
|
||||
@ -318,6 +337,17 @@ class LocalContainerProvisionerBackend:
|
||||
if container is None:
|
||||
return None
|
||||
container.reload()
|
||||
thread_id = str((container.labels or {}).get("thread-id") or "").strip()
|
||||
if not thread_id:
|
||||
return None
|
||||
safe_thread_id = self._validate_thread_id(thread_id)
|
||||
if not self._is_expected_skills_mount(container, safe_thread_id):
|
||||
logger.info("Discarding stale sandbox %s with legacy skills mount", sandbox_id)
|
||||
try:
|
||||
self.delete(sandbox_id)
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to delete stale sandbox %s during discover: %s", sandbox_id, exc)
|
||||
return None
|
||||
record = self._to_record(container, sandbox_id)
|
||||
if not record.sandbox_url:
|
||||
return None
|
||||
@ -403,16 +433,15 @@ class KubernetesProvisionerBackend:
|
||||
command=["sh", "-c"],
|
||||
args=[
|
||||
"chmod 777 /home/yuxi "
|
||||
"&& mkdir -p /home/yuxi/user-data/workspace /home/yuxi/user-data/uploads /home/yuxi/user-data/outputs "
|
||||
"&& chmod -R 777 /home/yuxi/user-data ",
|
||||
f"&& mkdir -p /mnt/shared-data/threads/{thread_id}/user-data/workspace "
|
||||
f"/mnt/shared-data/threads/{thread_id}/user-data/uploads "
|
||||
f"/mnt/shared-data/threads/{thread_id}/user-data/outputs "
|
||||
f"/mnt/shared-data/threads/{thread_id}/skills "
|
||||
f"&& chmod -R 777 /mnt/shared-data/threads/{thread_id}/user-data ",
|
||||
],
|
||||
volume_mounts=[
|
||||
self._client.V1VolumeMount(name="home-dir", mount_path="/home/yuxi"),
|
||||
self._client.V1VolumeMount(
|
||||
name="shared-data",
|
||||
mount_path="/home/yuxi/user-data",
|
||||
sub_path=f"threads/{thread_id}/user-data",
|
||||
),
|
||||
self._client.V1VolumeMount(name="shared-data", mount_path="/mnt/shared-data"),
|
||||
],
|
||||
),
|
||||
],
|
||||
@ -431,8 +460,8 @@ class KubernetesProvisionerBackend:
|
||||
self._client.V1VolumeMount(
|
||||
name="shared-data",
|
||||
mount_path="/home/yuxi/skills",
|
||||
sub_path="skills",
|
||||
read_only=False,
|
||||
sub_path=f"threads/{thread_id}/skills",
|
||||
read_only=True,
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
@ -212,17 +212,19 @@
|
||||
</a-tooltip> -->
|
||||
<p class="description">{{ database.description || '暂无描述' }}</p>
|
||||
<div class="tags">
|
||||
<a-tag color="blue" v-if="database.embed_info?.name">{{
|
||||
database.embed_info.name
|
||||
}}</a-tag>
|
||||
<!-- <a-tag color="green" v-if="database.embed_info?.dimension">{{ database.embed_info.dimension }}</a-tag> -->
|
||||
<a-tag
|
||||
:bordered="false"
|
||||
:color="getKbTypeColor(database.kb_type || 'lightrag')"
|
||||
class="kb-type-tag"
|
||||
size="small"
|
||||
>
|
||||
{{ getKbTypeLabel(database.kb_type || 'lightrag') }}
|
||||
</a-tag>
|
||||
<!-- 保留最后一个，使用 / 切分 -->
|
||||
<a-tag color="blue" v-if="database.embed_info?.name"
|
||||
:bordered="false">{{
|
||||
database.embed_info.name.split('/').slice(-1)[0]
|
||||
}}</a-tag>
|
||||
</div>
|
||||
<!-- <button @click="deleteDatabase(database.collection_name)">删除</button> -->
|
||||
</div>
|
||||
@ -675,11 +677,16 @@ onMounted(() => {
|
||||
|
||||
.database,
|
||||
.graphbase {
|
||||
background: linear-gradient(145deg, var(--gray-0) 0%, var(--gray-10) 100%);
|
||||
background: linear-gradient(45deg, var(--gray-0) 0%, var(--gray-25) 100%);
|
||||
box-shadow: 0px 1px 2px 0px var(--shadow-2);
|
||||
border: 1px solid var(--gray-100);
|
||||
transition: none;
|
||||
border: 1px solid var(--gray-50);
|
||||
transition: all 0.3s;
|
||||
position: relative;
|
||||
|
||||
&:hover {
|
||||
background: linear-gradient(45deg, var(--gray-0) 0%, var(--main-30) 100%);
|
||||
box-shadow: 0px 1px 5px var(--shadow-3);
|
||||
}
|
||||
}
|
||||
|
||||
.dbcard,
|
||||
@ -784,6 +791,10 @@ onMounted(() => {
|
||||
font-weight: 400;
|
||||
flex: 1;
|
||||
}
|
||||
|
||||
.tags {
|
||||
opacity: 0.8;
|
||||
}
|
||||
}
|
||||
|
||||
.database-empty {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user