diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index dd0393b8c..f2780dfb0 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -173,10 +173,12 @@ Box is the optional sandbox subsystem used by native execution tools, stdio MCP In this repo: -- `pkg/box/service.py` is the application-facing facade for exec, sessions, managed processes, skill CRUD, status, reconnects, quotas, mounts, and sandbox profiles. +- `pkg/box/service.py` is the application-facing facade for exec, sessions, managed processes, status, reconnects, quotas, generic mounts, and sandbox profiles. - `pkg/box/connector.py` connects to the Box Runtime over stdio, Windows subprocess+WebSocket, or remote WebSocket. -- `pkg/provider/tools/loaders/native.py`, `mcp_stdio.py`, and skill loaders depend on Box availability. -- `pkg/skill/repository.py` is the thin async/Workspace adapter over the Plugin SDK's execution-independent `SkillStore`; it preserves the existing `data/box/skills` layout shared with Box for optional execution mounts. +- `pkg/provider/tools/loaders/native.py` is the Core orchestration seam: the + Skill loader supplies generic read-only mounts to Box execution. `mcp_stdio.py` + and execution-backed tools depend on Box availability. +- `pkg/skill/repository.py` is the thin async/Workspace adapter over the Plugin SDK's execution-independent `SkillStore`; `skills.root` owns its location independently of Box. - `pkg/skill/manager.py` caches the Core repository catalog for progressive disclosure. Activation and read-only resource tools do not require Box; script execution and Workspace mutation still do. Durable Box Workspace storage is shared across placement generations, but @@ -188,11 +190,17 @@ retires stale processes and closes already-attached relays. In `langbot-plugin-sdk`: - `src/langbot_plugin/box/server.py` implements `lbp box` and the WebSocket endpoints on `:5410`. -- `src/langbot_plugin/box/runtime.py` owns sandbox sessions and managed processes. +- `src/langbot_plugin/box/runtime.py` owns sandbox sessions, generic read-only mounts, and managed processes. - `backend.py`, `nsjail_backend.py`, and `e2b_backend.py` implement sandbox backends. -- `skill_store.py` manages skill packages from the Box side. +- `src/langbot_plugin/skill_store.py` is consumed by Core, not Box. Core turns + selected package roots into generic read-only mounts; Box does not understand + Skill names, metadata, revisions, files, or CRUD. -Important config keys live under `box:` in `src/langbot/templates/config.yaml`: `box.enabled`, `box.backend`, `box.runtime.endpoint`, and `box.local.*`. Start LangBot with `--standalone-box` when connecting to an externally launched Box runtime. +Skill storage uses `skills.root`. Box execution config lives under `box:`: +`box.enabled`, `box.backend`, `box.runtime.endpoint`, and `box.local.*`. The old +`box.local.skills_root` key is read only as an online-upgrade fallback and is +marked for removal in the next major version. Start LangBot with +`--standalone-box` when connecting to an externally launched Box runtime. ## HTTP API, Web UI, and MCP Server diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index fa40faa40..757b7114d 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -96,7 +96,9 @@ services: # box.* and are forwarded to the Box runtime via INIT RPC. - BOX__LOCAL__HOST_ROOT=${LANGBOT_BOX_ROOT:-${PWD}/data/box} - BOX__LOCAL__DEFAULT_WORKSPACE=default - - BOX__LOCAL__SKILLS_ROOT=skills + # TODO(next-major): default LANGBOT_SKILLS_ROOT to ${PWD}/data/skills + # after the historical Box storage path no longer needs zero-copy upgrades. + - SKILLS__ROOT=${LANGBOT_SKILLS_ROOT:-${LANGBOT_BOX_ROOT:-${PWD}/data/box}/skills} - BOX__LOCAL__ALLOWED_MOUNT_ROOTS=${LANGBOT_BOX_ROOT:-${PWD}/data/box} - BOX__DOCKER__CPU_LIMIT_ENABLED=${LANGBOT_BOX_DOCKER_CPU_LIMIT_ENABLED:-true} ports: diff --git a/docker/kubernetes.yaml b/docker/kubernetes.yaml index 7ce145610..53a629e20 100644 --- a/docker/kubernetes.yaml +++ b/docker/kubernetes.yaml @@ -214,8 +214,9 @@ spec: # Deployment for LangBot Box (sandbox) runtime # # The Box runtime backs LangBot's sandbox tools (exec / read / write / edit / -# glob / grep), the `activate` skill tool, skill add/edit, and stdio-mode MCP -# servers. It is OPTIONAL: if you do not deploy it, set `BOX__ENABLED=false` on +# glob / grep), Skill script execution, and stdio-mode MCP servers. Skill +# activation, resources, and management remain Core-owned without Box. Box is +# OPTIONAL: if you do not deploy it, set `BOX__ENABLED=false` on # the langbot Deployment (or `box.enabled: false` in config.yaml) so the # dashboard renders cleanly with sandbox features disabled. # @@ -448,14 +449,15 @@ spec: key: token # box.local.* config — forwarded to the Box runtime via INIT RPC. The # host_root MUST match the box-root hostPath mountPath below AND the box - # Deployment's box-root mountPath, so that skill package paths resolve - # identically on both sides and on the node's Docker daemon. + # Deployment's box-root mountPath, so generic package mount paths + # resolve identically on both sides and on the node's Docker daemon. - name: BOX__LOCAL__HOST_ROOT value: "/app/data/box" - name: BOX__LOCAL__DEFAULT_WORKSPACE value: "default" - - name: BOX__LOCAL__SKILLS_ROOT - value: "skills" + - name: SKILLS__ROOT + # TODO(next-major): use /app/data/skills after the legacy path window. + value: "/app/data/box/skills" - name: BOX__LOCAL__ALLOWED_MOUNT_ROOTS value: "/app/data/box" volumeMounts: diff --git a/docs/review/box-architecture.md b/docs/review/box-architecture.md index 0f05b2852..b2bc3462f 100644 --- a/docs/review/box-architecture.md +++ b/docs/review/box-architecture.md @@ -22,6 +22,7 @@ │ │ │ (shared 容器, 多 process) │ │ │ │ │ │ │ ├──> SkillToolLoader (activate 工具) │ +│ │ │ └─ build_execution_mounts() │ │ │ │ │ │ │ ├──> SkillAuthoringToolLoader │ │ │ │ │ @@ -33,7 +34,7 @@ │ ├─ Workspace quota 检查 │ │ ├─ 输出截断 (head+tail) │ │ ├─ Session ID 模板解析 (resolve_box_session_id) │ -│ ├─ 技能挂载组装 (build_skill_extra_mounts) │ +│ ├─ 通用只读挂载接收 (read_only_mounts) │ │ ├─ 重连循环 (_reconnect_loop, 指数退避) │ │ └─ BoxRuntimeConnector │ │ ├─ 心跳 loop (20s ping) │ @@ -59,10 +60,8 @@ │ NsjailBackend ──┘ (本地 CLI 或 fallback 到容器内 CLI) │ │ E2BBackend (云沙箱, 需要 E2B_API_KEY) │ │ │ -│ BoxSkillStore │ -│ ├─ list / get / create / update / delete │ -│ ├─ scan_skill_directory / read_skill_file / write_skill_file │ -│ └─ preview_skill_zip / install_skill_zip (zip 或 GitHub) │ +│ Generic mount admission │ +│ └─ allow-list + read-only + normalized target validation │ │ │ │ aiohttp 单端口服务 (默认 :5410): │ │ /rpc/ws — Action RPC │ @@ -85,7 +84,7 @@ **核心设计原则**: - Box Runtime 作为独立进程运行，通过 Action RPC 与 LangBot 主进程通信，两者复用 SDK 的 IO 层（Handler → Connection → Controller） - 一个 session_id 对应一个容器/沙箱实例。同一 session 内可并存多条 mount 与多个 managed process -- Skill / 默认 exec / MCP Server 共享同一个 session 容器（详见 [box-session-scope.md](./box-session-scope.md)） +- Skill 仅是 Core 组装 mount 的业务来源；Box 与默认 exec / MCP Server 只共享通用 session 和 mount 机制（详见 [box-session-scope.md](./box-session-scope.md)） --- @@ -93,7 +92,7 @@ ### 2.1 BoxService (`pkg/box/service.py`, 722 行) -应用层门面，协调 Profile、安全校验、配额、连接、Skill 挂载与 Session 模板： +应用层门面，协调 Profile、安全校验、配额、连接、Core 生成的只读挂载与 Session 模板： 主要公开方法（按定义顺序）： @@ -105,7 +104,7 @@ BoxService ├─ available (property) 连接状态 │ ├─ resolve_box_session_id(query) 从 pipeline 模板解析 session_id - ├─ build_skill_extra_mounts(query) 组装 pipeline-bound skill 的挂载列表 + ├─ execute_tool(..., read_only_mounts=...) 接收 Core 组装的通用只读挂载 │ ├─ execute_tool(parameters, query) Agent 调用 exec 时的入口 │ ├─ _apply_profile / build_spec @@ -122,12 +121,6 @@ BoxService ├─ stop_managed_process(session_id, pid) 单独停止某个 managed process ├─ get_managed_process_websocket_url(...) 返回 WS attach URL │ - ├─ list_skills() / get_skill(name) Skill 元数据 - ├─ create_skill / update_skill / delete_skill Skill CRUD - ├─ scan_skill_directory(path) 扫描目录 - ├─ list_skill_files / read_skill_file / write_skill_file - ├─ preview_skill_zip / install_skill_zip zip / GitHub 安装 - │ ├─ shutdown() / dispose() 清理：RPC SHUTDOWN + 进程终止 ├─ get_status() / get_sessions() / get_recent_errors() └─ get_system_guidance() LLM 系统提示 @@ -137,7 +130,7 @@ BoxService **输出截断**: 默认 4000 字符上限，保留前 60% + 后 40%，中间插入 `[...truncated...]`。 -**Skill 挂载合并**: `execute_tool()` 调用时，`build_skill_extra_mounts(query)` 会把当前 pipeline-bound 的所有 skill 的 `package_root` 作为 `extra_mounts` 加入 BoxSpec，挂在 `/workspace/.skills/`。LLM 通过 `activate` 工具显式激活某个 skill 后，工具调用才允许引用这个 skill 的虚拟路径。 +**Skill 挂载合并**: native loader 调用 `skill.build_execution_mounts()`，把当前 pipeline-bound 的所有 skill 的 `package_root` 转成普通只读 mount，再通过 `BoxService.execute_tool(..., read_only_mounts=...)` 交给 Box，挂在 `/workspace/.skills/`。LLM 通过 `activate` 工具显式激活某个 skill 后，工具调用才允许引用这个 skill 的虚拟路径；BoxService 和 Box Runtime 都不知道这些 mount 来自 Skill。 ### 2.2 BoxRuntimeConnector (`pkg/box/connector.py`, 357 行) @@ -294,7 +287,7 @@ start_managed_process(session, spec): 单端口 aiohttp 服务（默认 5410），通过路径区分（commit `8c71ec5` 合并端口）： -1. **Action RPC** (`/rpc/ws`): `BoxServerHandler` 处理所有 action，包括 `INIT` 配置注入、skill store 操作等 +1. **Action RPC** (`/rpc/ws`): `BoxServerHandler` 处理 `INIT`、exec、session、managed-process 与状态等通用 action 2. **WS Relay** (`/v1/sessions/{id}/managed-process/ws` 与 `/v1/sessions/{id}/managed-process/{pid}/ws`): 双向桥接 WebSocket ↔ 指定 managed process stdin/stdout stdio 模式同样会在 5410 启动 aiohttp，专门承担 managed process attach；Action RPC 走 stdin/stdout。 @@ -303,7 +296,7 @@ stdio 模式同样会在 5410 启动 aiohttp，专门承担 managed process atta `ActionRPCBoxClient` 封装 `Handler.call_action()` 调用： -- 25+ 方法对应 25+ 个 RPC action（exec / session / managed-process / skill / status / shutdown） +- 方法对应 exec / session / managed-process / status / shutdown 等通用 RPC action，不暴露 Skill CRUD - 错误还原: `_translate_action_error()` 通过字符串前缀匹配还原 SDK 侧异常类型 - `execute()` timeout = 300s，其他默认 15s - `BoxRuntimeClient` 是 ABC，供后续可能的非 RPC 实现复用 @@ -344,7 +337,7 @@ stdio 模式同样会在 5410 启动 aiohttp，专门承担 managed process atta ### 3.7 SkillStore (`langbot_plugin.skill_store`) -Skill 包存储最初位于 Box Runtime；issue #2410 将通用实现抽到 Plugin SDK 顶层，Core 与 Box 使用同一套存储和 revision 语义： +Skill 包存储最初位于 Box Runtime；issue #2410 将通用实现抽到 Plugin SDK 顶层，由 Core 独占存储和 revision 语义： ``` SkillStore @@ -359,7 +352,15 @@ SkillStore └─ 支持 source_subdir / target_suffix（commit 1aa043f） ``` -`langbot_plugin.box.skill_store.BoxSkillStore` 仅保留为旧 Box 配置到通用 `SkillStore` 的兼容适配器，不再拥有存储实现。GitHub 安装路径由 Core HTTP 层下载归档，再交给 SkillRepository。Skill 文件继续存放于 `box.local.skills_root`（默认 `skills`，相对 `host_root`），执行时只读挂载到 `/workspace/.skills/`。 +GitHub 安装路径由 Core HTTP 层下载归档，再交给 SkillRepository。Skill 文件位于独立的 `skills.root`，执行时由 Core 组装成通用只读 `BoxMountSpec` 并挂载到 `/workspace/.skills/`。Box 的正常模型、客户端和 Runtime 不包含 `skill_name`、Skill CRUD、revision 或 `SKILL.md` 语义。 + +滚动升级只保留一个隔离桥：`box/legacy_skill_compat.py` 让旧 Core 暂时调用新 Box，并把旧 `skill_name` 转为普通只读 mount。部署顺序必须先升级 Box、再升级 Core。该模块有 `TODO(next-major)`，下一大版本删除；正常架构不依赖它。 + +下一大版本的删除清单（当前均有 `TODO(next-major)`）： + +1. 删除 SDK `box/legacy_skill_compat.py` 及 Server 中唯一的注册/转换钩子。 +2. 删除 Core 对 `box.local.skills_root` 的配置 fallback。 +3. 新安装默认从历史 `./data/box/skills` 切到 `./data/skills`；届时 Box 部署只需只读访问 Core 明确下发的通用 artifact root。 ### 3.8 Security (`box/security.py`, 52 行) @@ -504,6 +505,9 @@ Box 额外做了 RPC SHUTDOWN 通知 Runtime 主动清理容器，比 Plugin 的 ### config.yaml (重构后) ```yaml +skills: + root: './data/box/skills' # Core-owned；Box 关闭时仍可管理/读取 + box: enabled: true # 整个 Box 子系统的总开关。设为 false 时： # - 不连接远程 Box runtime，不 fork 本地 stdio 子进程 @@ -522,7 +526,6 @@ box: image: '' # 覆盖 profile 默认 image host_root: './data/box' # 工作区挂载根，Docker 部署需绝对路径 default_workspace: '' # 默认 '/default' - skills_root: 'skills' # Core 管理且与 Box 共享的 skill 包目录 allowed_mount_roots: # 默认 [''] - './data/box' - '/tmp' @@ -571,7 +574,7 @@ volumes: | Pipeline 扩展页 `enable_all_skills` / 绑定 skill | 可编辑 | 可编辑 | | 仪表盘 Box 状态卡片 | 绿点 / "已连接" | 灰点 / "已禁用"(disabled) 或 红点 / "已断开"(failed) | -> Core 的 SkillRepository 是 `langbot_plugin.skill_store.SkillStore` 的异步 Workspace 适配层，保持原 `data/box/skills/tenants/...` 布局，升级时无需移动已安装 Skill。Box 只在执行发生时消费同一份只读 package revision。 +> Core 的 SkillRepository 是 `langbot_plugin.skill_store.SkillStore` 的异步 Workspace 适配层。默认 `skills.root` 保持原 `data/box/skills/tenants/...` 布局，升级时无需移动已安装 Skill；旧 `box.local.skills_root` 仅作为在线升级 fallback，并将在下一大版本删除。Box 只消费 Core 下发的通用只读 mount。 ### Pipeline 配置 (templates/metadata/pipeline/ai.yaml) diff --git a/docs/review/box-issues.md b/docs/review/box-issues.md index 15650c7c5..7c3922499 100644 --- a/docs/review/box-issues.md +++ b/docs/review/box-issues.md @@ -52,8 +52,8 @@ ### S5. 挂载校验缺口 — Med-High - **位置**: SDK `box/security.py` `_BLOCKED_HOST_PATHS_POSIX`；`box/backend.py` 的 `extra_mounts` 处理 -- **现状**: ① SDK 黑名单仍不含 `/`（前缀匹配，`host_path="/"` 可通过，挂载整个宿主 fs）；用户 home、`/usr`、`/opt`、`/tmp` 也未拦截。② `validate_sandbox_security` 只校验 `spec.host_path`，**从不遍历 `spec.extra_mounts`**——LangBot 侧 `allowed_mount_roots` 也只校验 `host_path`。当前 `extra_mounts` 仅由 `build_skill_extra_mounts` 内部填充（agent 不可达），但缺乏纵深防御：一旦 S1 的无认证 RPC 被触达，extra_mounts 可挂任意宿主路径，两层都不拦。 -- **要求**: SDK 黑名单加入 `/`（或改白名单）；`extra_mounts` 在 SDK 与 LangBot 两侧都纳入挂载校验。 +- **现状**: grant-enforced 模式已经由 Core 与 Runtime 双重校验通用只读 mount（绝对路径 allow-list、存在性、只读模式、规范化且位于 `/workspace` 下的目标）；它不再有 Skill 特例。遗留风险仅在 admission-disabled 的低信任直连场景：通用 `extra_mounts` 仍未统一套用 grant-enforced 白名单。 +- **要求**: admission-disabled 的外部控制面也复用同一套通用 mount 校验；SDK 黑名单加入 `/`（或全面改白名单）。 ### S6. 容器加固缺失 — Med diff --git a/docs/review/box-session-scope.md b/docs/review/box-session-scope.md index bb92265de..1d76b7fc6 100644 --- a/docs/review/box-session-scope.md +++ b/docs/review/box-session-scope.md @@ -18,7 +18,7 @@ has shipped the design largely as written: | Docker / nsjail / E2B backends apply extra mounts | ✅ Shipped | Last gap closed by SDK commit `0fea9b1` (E2B) | | `box-session-id-template` in `local-agent` pipeline config | ✅ Shipped | `templates/metadata/pipeline/ai.yaml`, default `{launcher_type}_{launcher_id}` | | `BoxService.resolve_box_session_id(query)` | ✅ Shipped | `pkg/box/service.py:166` | -| `BoxService.build_skill_extra_mounts(query)` | ✅ Shipped | `pkg/box/service.py:189` | +| `skill.build_execution_mounts(ap, query)` | ✅ Shipped | Core composes read-only packages; Box receives generic mounts | | Skill exec uses unified container + extra mounts | ✅ Shipped | `pkg/provider/tools/loaders/native.py` skill branch | | MCP-in-Box uses shared persistent session, multi-process | ✅ Shipped (earlier than originally scoped) | SDK commit `529088e`, LangBot `mcp_stdio.py:_build_box_session_id` | | `BoxManagedProcessSpec.process_id` + multi-process per session | ✅ Shipped | `BoxRuntime` keeps `managed_processes: dict[pid, _ManagedProcess]` | diff --git a/docs/review/box-test-coverage.md b/docs/review/box-test-coverage.md index 995e6970b..57d6d90a6 100644 --- a/docs/review/box-test-coverage.md +++ b/docs/review/box-test-coverage.md @@ -51,7 +51,7 @@ | BoxService workspace quota | 优秀 | 前置/后置配额检查、超额清理 | | BoxService 输出截断 | 优秀 | 短/精确边界/长输出、独立 stderr | | BoxService 可观测性 | 优秀 | 状态报告、error ring buffer、buffer 上限 | -| BoxService session 模板 | 良好 | `resolve_box_session_id` + `build_skill_extra_mounts` 在 service / native / mcp 三处都有覆盖 | +| BoxService session / mount contract | 良好 | `resolve_box_session_id` + generic `read_only_mounts`; Skill mount composition is covered in the Core loader | | RPC client/server 协议 | 优秀 | execute/get_sessions/delete/create/conflict error | | BoxRuntimeConnector | 良好 | local/remote 模式、Docker 平台、relay URL、心跳与重连回调 | | BoxWorkspaceSession | 良好 | payload 构建、managed process 路径重写、stage host file | diff --git a/src/langbot/pkg/box/connector.py b/src/langbot/pkg/box/connector.py index e10e8b9ac..f82f3b425 100644 --- a/src/langbot/pkg/box/connector.py +++ b/src/langbot/pkg/box/connector.py @@ -147,10 +147,9 @@ class BoxRuntimeConnector(ManagedRuntimeConnector): - An explicit ``runtime.endpoint`` was configured When this is True the Box runtime lives in a separate process with its - own filesystem view (container, pod sidecar, or remote host), so paths - it reports (e.g. skill ``package_root``) are NOT resolvable on the - LangBot side. When False, Box runs as a stdio child process that shares - LangBot's filesystem. + own filesystem view (container, pod sidecar, or remote host), so only + explicitly shared paths are usable on both sides. When False, Box runs + as a stdio child process that shares LangBot's filesystem. """ return bool( self.configured_runtime_endpoint diff --git a/src/langbot/pkg/box/service.py b/src/langbot/pkg/box/service.py index 72ffa6b2a..50eb29cab 100644 --- a/src/langbot/pkg/box/service.py +++ b/src/langbot/pkg/box/service.py @@ -28,8 +28,10 @@ from langbot_plugin.box.errors import BoxAdmissionError, BoxError, BoxValidation from langbot_plugin.box.models import ( BUILTIN_PROFILES, BoxExecutionResult, + BoxHostMountMode, BoxManagedProcessInfo, BoxManagedProcessSpec, + BoxMountSpec, BoxProfile, BoxSpec, ) @@ -169,7 +171,7 @@ class BoxService: self._connector_error = 'Box runtime is disabled in config (box.enabled = false)' self.ap.logger.info( 'Box runtime disabled by config; sandbox features (exec/read/write/edit, ' - 'skill add/edit, stdio MCP) will be unavailable.' + 'stdio MCP, executable package scripts) will be unavailable.' ) return try: @@ -275,10 +277,6 @@ class BoxService: await self._purge_attachment_dirs() self._available = True self._connector_error = '' - skill_mgr = getattr(self.ap, 'skill_mgr', None) - reload_skills = getattr(skill_mgr, 'reload_skills', None) - if callable(reload_skills) and not self._cloud_managed: - await reload_skills() self.ap.logger.info('Box runtime reconnected, sandbox features restored.') return except Exception as exc: @@ -358,19 +356,17 @@ class BoxService: """Whether LangBot and the Box runtime share a filesystem view. This is True only when Box runs as a local stdio child process of - LangBot (same container/host). In that case paths the Box runtime - reports — notably skill ``package_root`` — resolve identically on the - LangBot side, so LangBot may validate them against its own filesystem. + LangBot (same container/host). In that case host paths resolve + identically on both sides and Core may perform local filesystem work. It is False for every separated deployment (Docker Compose, k8s sidecar, ``--standalone-box``, or an explicit ``runtime.endpoint``), - where the Box runtime owns its own filesystem and LangBot must trust - the paths it reports rather than checking them locally. + where only explicitly shared and identically mounted roots can cross + the process boundary. When Box is wired up with an injected client (tests, custom embeds) - there is no connector to introspect; we conservatively report False so - LangBot never wrongly drops Box-reported skills. An explicit override - can be set via ``_shares_filesystem_with_box`` (used by tests and any + there is no connector to introspect; we conservatively report False. + An explicit override can be set via ``_shares_filesystem_with_box`` (used by tests and any embedder that knows the real topology). """ if self._shares_filesystem_with_box_override is not None: @@ -483,11 +479,22 @@ class BoxService: self, context: TenantContext, spec_payload: dict, + *, + trusted_read_only_mounts: list[dict] | None = None, ) -> dict: """Reject tenant-owned policy fields and apply the Cloud hard policy.""" payload = dict(spec_payload) + trusted_mounts = self._normalize_trusted_read_only_mounts( + trusted_read_only_mounts or [] + ) if not self._cloud_managed: + if trusted_mounts: + if payload.get('extra_mounts'): + raise BoxValidationError( + 'extra_mounts and trusted_read_only_mounts cannot both be supplied' + ) + payload['extra_mounts'] = trusted_mounts return payload policy = self._admission_policy if policy is None: @@ -537,7 +544,7 @@ class BoxService: 'network': 'off', 'host_path': canonical_host_path, 'mount_path': '/workspace', - 'extra_mounts': [], + 'extra_mounts': trusted_mounts, 'persistent': True, 'timeout_sec': min(timeout, policy.max_timeout_sec), 'cpus': policy.cpus, @@ -549,6 +556,26 @@ class BoxService: ) return payload + def _normalize_trusted_read_only_mounts(self, mounts: list[dict]) -> list[dict]: + """Validate Core-composed artifacts before crossing into Box.""" + + normalized: list[dict] = [] + for raw_mount in mounts: + mount = BoxMountSpec.model_validate(raw_mount) + if mount.mode != BoxHostMountMode.READ_ONLY: + raise BoxAdmissionError('Core-composed additional mounts must be read-only') + host_path = os.path.realpath(mount.host_path) + if not os.path.isdir(host_path): + raise BoxAdmissionError('Core-composed read-only mount source is unavailable') + if not any(_is_path_under(host_path, root) for root in self.allowed_mount_roots): + raise BoxAdmissionError( + 'Core-composed read-only mount source is outside allowed_mount_roots' + ) + normalized.append( + mount.model_copy(update={'host_path': host_path}).model_dump(mode='json') + ) + return normalized + def _reject_cloud_managed_process(self) -> None: if self._cloud_managed: raise BoxAdmissionError('Managed processes are disabled for Cloud sandboxes') @@ -565,13 +592,18 @@ class BoxService: query: pipeline_query.Query, *, skip_host_mount_validation: bool = False, + trusted_read_only_mounts: list[dict] | None = None, ) -> dict: if not self._available: raise BoxError( 'Box runtime is not available. Configure an available Box backend before using Box features.' ) execution_context = await self._validated_execution_context(self._query_execution_context(query)) - spec_payload = self._managed_policy_payload(execution_context, spec_payload) + spec_payload = self._managed_policy_payload( + execution_context, + spec_payload, + trusted_read_only_mounts=trusted_read_only_mounts, + ) await self._require_validated_workspace_sandbox(execution_context) if spec_payload.get('host_path') in (None, ''): tenant_workspace = self._tenant_workspace(execution_context) @@ -658,70 +690,12 @@ class BoxService: variables.setdefault('global', 'global') return template.format_map(collections.defaultdict(lambda: 'unknown', variables)) - def build_skill_extra_mounts(self, query: pipeline_query.Query) -> list[dict]: - """Build extra_mounts entries for all pipeline-bound skills. - - This ensures that when a container is first created it already has - all skill packages mounted, regardless of which skill is currently - activated. - - Path validation is filesystem-topology dependent. When LangBot and the - Box runtime share a filesystem (local stdio mode), a skill whose - ``package_root`` is missing or no longer a directory is skipped with a - warning instead of being passed through to the backend. Without that - guard the three backends behave inconsistently on a stale mount: nsjail - refuses to start the sandbox (failing every exec in the session), - Docker silently auto-creates a root-owned empty directory on the host, - and E2B silently skips the upload — none of which surfaces an - actionable error. - - When Box runs as a separate process (Docker Compose, k8s sidecar, - ``--standalone-box``, or a remote ``runtime.endpoint``), the - ``package_root`` reported by ``list_skills`` is the Box runtime's own - filesystem path and is NOT resolvable on the LangBot side. Validating - it locally would wrongly drop every skill, so LangBot trusts the path - and lets the Box runtime resolve it. The Box runtime only ever reports - skills it discovered on its own filesystem, so the path is valid there - by construction. - """ - if self._cloud_managed: - return [] - skill_mgr = getattr(self.ap, 'skill_mgr', None) - if skill_mgr is None: - return [] - - from ..provider.tools.loaders import skill as skill_loader - - validate_locally = self.shares_filesystem_with_box - - visible_skills = skill_loader.get_visible_skills(self.ap, query) - mounts: list[dict] = [] - for skill_name, skill_data in visible_skills.items(): - package_root = str(skill_data.get('package_root', '') or '').strip() - if not package_root: - continue - if validate_locally and not os.path.isdir(package_root): - self.ap.logger.warning( - f'Skill "{skill_name}" package_root missing on filesystem ' - f'({package_root}); skipping mount to prevent sandbox failures. ' - f'The skill cache may be stale — consider reloading skills.' - ) - continue - mounts.append( - { - 'host_path': package_root, - 'mount_path': f'/workspace/.skills/{skill_name}', - 'mode': 'ro', - } - ) - return mounts - async def execute_tool( self, parameters: dict, query: pipeline_query.Query, *, - skill_name: str | None = None, + read_only_mounts: list[dict] | None = None, ) -> dict: """Execute an agent-facing ``exec`` tool call. @@ -729,8 +703,6 @@ class BoxService: ``BoxSpec.cmd`` field and injects the session id from the query. """ spec_payload: dict = {'cmd': parameters['command']} - if skill_name is not None: - spec_payload['skill_name'] = skill_name # Pass through allowed agent-facing fields for key in ('workdir', 'timeout_sec', 'env'): @@ -740,11 +712,11 @@ class BoxService: # Inject context the agent must not control spec_payload.setdefault('session_id', self.resolve_box_session_id(query)) - # Mount all pipeline-bound skills so they are available in the container - if 'extra_mounts' not in spec_payload: - spec_payload['extra_mounts'] = self.build_skill_extra_mounts(query) - - return await self.execute_spec_payload(spec_payload, query) + return await self.execute_spec_payload( + spec_payload, + query, + trusted_read_only_mounts=read_only_mounts, + ) async def execute_in_context( self, @@ -1275,8 +1247,6 @@ class BoxService: 'timeout_sec': 120, 'session_id': self.resolve_box_session_id(query), } - if 'extra_mounts' not in spec_payload: - spec_payload['extra_mounts'] = self.build_skill_extra_mounts(query) try: spec = self.build_spec(spec_payload) result = await self.client.execute(spec) @@ -1527,126 +1497,6 @@ class BoxService: self._runtime_connector.get_relay_headers(action_context), ) - async def list_skills(self, context: TenantContext) -> list[dict]: - execution_context = await self._validated_skill_execution_context(context) - return await self.client.list_skills(action_context=self._action_context(execution_context)) - - async def get_skill(self, context: TenantContext, name: str) -> dict | None: - execution_context = await self._validated_skill_execution_context(context) - return await self.client.get_skill(name, action_context=self._action_context(execution_context)) - - async def create_skill(self, context: TenantContext, skill: dict) -> dict: - execution_context = await self._validated_skill_execution_context(context) - payload = dict(skill) - payload.pop('workspace_uuid', None) - if self._cloud_managed and str(payload.get('package_root', '') or '').strip(): - raise BoxAdmissionError('Cloud skill package_root is runtime-owned') - if self._cloud_managed: - payload.pop('package_root', None) - return await self.client.create_skill(payload, action_context=self._action_context(execution_context)) - - async def update_skill(self, context: TenantContext, name: str, skill: dict) -> dict: - execution_context = await self._validated_skill_execution_context(context) - payload = dict(skill) - payload.pop('workspace_uuid', None) - if self._cloud_managed: - # The runtime already owns the package path for an existing skill. - # A serialized read response may contain it, but it is never an - # authority-bearing update field in shared Cloud mode. - payload.pop('package_root', None) - return await self.client.update_skill( - name, - payload, - action_context=self._action_context(execution_context), - ) - - async def delete_skill(self, context: TenantContext, name: str) -> None: - execution_context = await self._validated_skill_execution_context(context) - await self.client.delete_skill(name, action_context=self._action_context(execution_context)) - - async def scan_skill_directory(self, context: TenantContext, path: str) -> dict: - execution_context = await self._validated_skill_execution_context(context) - if self._cloud_managed: - raise BoxAdmissionError('Scanning arbitrary host skill directories is disabled in Cloud') - return await self.client.scan_skill_directory(path, action_context=self._action_context(execution_context)) - - async def _validated_skill_execution_context(self, context: TenantContext) -> ExecutionContext: - execution_context = await self._validated_execution_context(context) - await self._require_validated_workspace_sandbox(execution_context) - return execution_context - - async def list_skill_files( - self, - context: TenantContext, - name: str, - path: str = '.', - include_hidden: bool = False, - max_entries: int = 200, - ) -> dict: - execution_context = await self._validated_skill_execution_context(context) - return await self.client.list_skill_files( - name, - path, - include_hidden, - max_entries, - action_context=self._action_context(execution_context), - ) - - async def read_skill_file(self, context: TenantContext, name: str, path: str) -> dict: - execution_context = await self._validated_skill_execution_context(context) - return await self.client.read_skill_file( - name, - path, - action_context=self._action_context(execution_context), - ) - - async def write_skill_file(self, context: TenantContext, name: str, path: str, content: str) -> dict: - execution_context = await self._validated_skill_execution_context(context) - return await self.client.write_skill_file( - name, - path, - content, - action_context=self._action_context(execution_context), - ) - - async def preview_skill_zip( - self, - context: TenantContext, - file_bytes: bytes, - filename: str, - source_subdir: str = '', - target_suffix: str = 'upload', - ) -> list[dict]: - execution_context = await self._validated_skill_execution_context(context) - return await self.client.preview_skill_zip( - file_bytes, - filename, - source_subdir, - target_suffix, - action_context=self._action_context(execution_context), - ) - - async def install_skill_zip( - self, - context: TenantContext, - file_bytes: bytes, - filename: str, - source_paths: list[str] | None = None, - source_path: str = '', - source_subdir: str = '', - target_suffix: str = 'upload', - ) -> list[dict]: - execution_context = await self._validated_skill_execution_context(context) - return await self.client.install_skill_zip( - file_bytes, - filename, - source_paths, - source_path, - source_subdir, - target_suffix, - action_context=self._action_context(execution_context), - ) - def _serialize_result(self, result: BoxExecutionResult) -> dict: stdout, stdout_truncated = self._truncate(result.stdout) stderr, stderr_truncated = self._truncate(result.stderr) @@ -1783,14 +1633,6 @@ class BoxService: default_workspace = os.path.join(self.host_root, default_workspace) return os.path.realpath(os.path.abspath(default_workspace)) - def get_skills_root(self) -> str | None: - skills_root = str(self._local_config().get('skills_root', '') or 'skills').strip() - if not skills_root: - skills_root = 'skills' - if not os.path.isabs(skills_root) and self.host_root is not None: - skills_root = os.path.join(self.host_root, skills_root) - return os.path.realpath(os.path.abspath(skills_root)) - def _load_enabled(self) -> bool: """Read ``box.enabled`` (top-level, not ``box.local.*``). Default True — disabling is opt-in. Accepts bool, ``'true'``/``'false'`` strings, diff --git a/src/langbot/pkg/box/workspace.py b/src/langbot/pkg/box/workspace.py index 3dd91c67b..2e1357f62 100644 --- a/src/langbot/pkg/box/workspace.py +++ b/src/langbot/pkg/box/workspace.py @@ -1,43 +1,27 @@ """Reusable workspace/session helpers built on top of Box. -This module is the middle layer between the raw Box runtime primitives and -application-specific flows such as skills or MCP stdio. +This module is the middle layer between raw Box runtime primitives and +application-specific consumers. It intentionally stays generic: - path and virtualenv rewriting are workspace concerns - Python project detection/bootstrap are workspace concerns - session exec / managed-process helpers are workspace concerns -Higher layers add their own semantics on top, for example: -- skills choose a stable per-skill session id and use repeated exec -- MCP stdio chooses how to prepare dependencies and attaches to a managed process +Higher layers add their own semantics on top; BoxWorkspaceSession retains only +workspace, execution, and managed-process concepts. """ from __future__ import annotations import os -import textwrap from typing import Any -PYTHON_MANIFEST_FILES = ( - 'requirements.txt', - 'pyproject.toml', - 'setup.py', - 'setup.cfg', -) +from ..utils.python_workspace import list_python_manifest_files _VENV_DIRS = frozenset({'.venv', 'venv', 'env', '.env'}) _VENV_BIN_DIRS = frozenset({'bin', 'Scripts'}) -def normalize_host_path(path: str | None) -> str: - if path is None: - return '' - stripped = str(path).strip() - if not stripped: - return '' - return os.path.realpath(os.path.abspath(stripped)) - - def rewrite_mounted_path(path: str, host_path: str | None, *, mount_path: str = '/workspace') -> str: """Translate a host path into the path visible inside the sandbox mount.""" if not host_path or not path: @@ -98,13 +82,6 @@ def rewrite_venv_command(command: str, host_path: str | None, *, mount_path: str return rewrite_mounted_path(normalized_command, host_path, mount_path=mount_path) -def list_python_manifest_files(host_path: str | None) -> list[str]: - normalized_root = normalize_host_path(host_path) - if not normalized_root: - return [] - return [filename for filename in PYTHON_MANIFEST_FILES if os.path.isfile(os.path.join(normalized_root, filename))] - - def classify_python_workspace(host_path: str | None) -> str | None: """Return the generic Python workspace shape, without app-specific policy.""" manifest_files = set(list_python_manifest_files(host_path)) @@ -117,161 +94,6 @@ def classify_python_workspace(host_path: str | None) -> str | None: return None -def should_prepare_python_env(host_path: str | None) -> bool: - normalized_root = normalize_host_path(host_path) - if not normalized_root: - return False - if os.path.isdir(os.path.join(normalized_root, '.venv')): - return True - return bool(list_python_manifest_files(normalized_root)) - - -def wrap_python_command_with_env( - command: str, - *, - mount_path: str = '/workspace', - state_path: str | None = None, -) -> str: - """Wrap a command with a reusable sandbox-local Python env bootstrap. - - ``mount_path`` is always the source tree used for manifest hashing and - installation. ``state_path`` may point at a separate writable directory - for read-only source mounts; when omitted, legacy mutable-workspace behavior - stores the environment beside the source. - """ - writable_state_path = state_path or mount_path - bootstrap = textwrap.dedent( - f""" - set -e - - _LB_VENV_DIR="{writable_state_path}/.venv" - _LB_META_DIR="{writable_state_path}/.langbot" - _LB_META_FILE="$_LB_META_DIR/python-env.json" - _LB_LOCK_DIR="$_LB_META_DIR/python-env.lock" - _LB_TMP_DIR="{writable_state_path}/.tmp" - _LB_PIP_CACHE_DIR="{writable_state_path}/.cache/pip" - - mkdir -p "$_LB_META_DIR" "$_LB_TMP_DIR" "$_LB_PIP_CACHE_DIR" - _LB_SYSTEM_PYTHON="$(command -v python3 || command -v python || true)" - if [ -z "$_LB_SYSTEM_PYTHON" ]; then - echo "python3 or python is required to prepare the workspace Python environment" >&2 - exit 127 - fi - - export TMPDIR="$_LB_TMP_DIR" - export TEMP="$_LB_TMP_DIR" - export TMP="$_LB_TMP_DIR" - export PIP_CACHE_DIR="$_LB_PIP_CACHE_DIR" - - _lb_python_meta() {{ - "$_LB_SYSTEM_PYTHON" - <<'PY' - import hashlib - import json - import os - import sys - - root = "{mount_path}" - max_manifest_bytes = 10 * 1024 * 1024 - digest = hashlib.sha256() - manifest_files = [] - for rel in ("requirements.txt", "pyproject.toml", "setup.py", "setup.cfg"): - path = os.path.join(root, rel) - if not os.path.isfile(path): - continue - if os.path.getsize(path) > max_manifest_bytes: - raise RuntimeError( - f"Python project manifest exceeds {{max_manifest_bytes}} bytes: {{rel}}" - ) - manifest_files.append(rel) - with open(path, "rb") as handle: - digest.update(rel.encode("utf-8")) - digest.update(b"\\0") - while chunk := handle.read(1024 * 1024): - digest.update(chunk) - digest.update(b"\\0") - - print( - json.dumps( - {{ - "python_executable": sys.executable, - "python_version": list(sys.version_info[:3]), - "manifest_files": manifest_files, - "manifest_sha256": digest.hexdigest(), - }}, - sort_keys=True, - ) - ) - PY - }} - - _LB_CURRENT_META="$(_lb_python_meta)" - _LB_NEEDS_BOOTSTRAP=0 - - if [ ! -x "$_LB_VENV_DIR/bin/python" ]; then - _LB_NEEDS_BOOTSTRAP=1 - elif [ ! -f "$_LB_META_FILE" ]; then - _LB_NEEDS_BOOTSTRAP=1 - elif [ "$(cat "$_LB_META_FILE")" != "$_LB_CURRENT_META" ]; then - _LB_NEEDS_BOOTSTRAP=1 - fi - - if [ "$_LB_NEEDS_BOOTSTRAP" -eq 1 ]; then - _LB_LOCK_WAIT=0 - while ! mkdir "$_LB_LOCK_DIR" 2>/dev/null; do - if [ "$_LB_LOCK_WAIT" -ge 120 ]; then - _LB_LOCK_OWNER="$(cat "$_LB_LOCK_DIR/pid" 2>/dev/null || true)" - if [ -n "$_LB_LOCK_OWNER" ] && kill -0 "$_LB_LOCK_OWNER" 2>/dev/null; then - echo "Timed out waiting for active Python environment lock: $_LB_LOCK_DIR" >&2 - exit 1 - fi - echo "Timed out waiting for Python environment lock, clearing stale lock: $_LB_LOCK_DIR" >&2 - rm -rf "$_LB_LOCK_DIR" 2>/dev/null || true - if mkdir "$_LB_LOCK_DIR" 2>/dev/null; then - break - fi - echo "Timed out waiting for Python environment lock: $_LB_LOCK_DIR" >&2 - exit 1 - fi - sleep 1 - _LB_LOCK_WAIT=$((_LB_LOCK_WAIT + 1)) - done - printf '%s\\n' "$$" > "$_LB_LOCK_DIR/pid" 2>/dev/null || true - - _lb_cleanup_lock() {{ - rm -rf "$_LB_LOCK_DIR" >/dev/null 2>&1 || true - }} - trap _lb_cleanup_lock EXIT INT TERM - - _LB_CURRENT_META="$(_lb_python_meta)" - _LB_NEEDS_BOOTSTRAP=0 - if [ ! -x "$_LB_VENV_DIR/bin/python" ]; then - _LB_NEEDS_BOOTSTRAP=1 - elif [ ! -f "$_LB_META_FILE" ]; then - _LB_NEEDS_BOOTSTRAP=1 - elif [ "$(cat "$_LB_META_FILE")" != "$_LB_CURRENT_META" ]; then - _LB_NEEDS_BOOTSTRAP=1 - fi - - if [ "$_LB_NEEDS_BOOTSTRAP" -eq 1 ]; then - rm -rf "$_LB_VENV_DIR" - "$_LB_SYSTEM_PYTHON" -m venv "$_LB_VENV_DIR" - . "$_LB_VENV_DIR/bin/activate" - python -m pip install --upgrade pip setuptools wheel - if [ -f "{mount_path}/requirements.txt" ]; then - python -m pip install -r "{mount_path}/requirements.txt" - elif [ -f "{mount_path}/pyproject.toml" ] || [ -f "{mount_path}/setup.py" ] || [ -f "{mount_path}/setup.cfg" ]; then - python -m pip install "{mount_path}" - fi - printf '%s' "$_LB_CURRENT_META" > "$_LB_META_FILE" - fi - fi - - export VIRTUAL_ENV="$_LB_VENV_DIR" - export PATH="$_LB_VENV_DIR/bin:$PATH" - {command} - """ - ).strip() - return bootstrap + '\n' class BoxWorkspaceSession: diff --git a/src/langbot/pkg/provider/tools/loaders/mcp_stdio.py b/src/langbot/pkg/provider/tools/loaders/mcp_stdio.py index 110fff431..36d2f4473 100644 --- a/src/langbot/pkg/provider/tools/loaders/mcp_stdio.py +++ b/src/langbot/pkg/provider/tools/loaders/mcp_stdio.py @@ -18,12 +18,11 @@ from ....box.workspace import ( BoxWorkspaceSession, classify_python_workspace, infer_workspace_host_path, - normalize_host_path, rewrite_mounted_path, rewrite_venv_command, unwrap_venv_path, - wrap_python_command_with_env, ) +from ....utils.python_workspace import normalize_host_path, wrap_python_command_with_env if TYPE_CHECKING: from .mcp import RuntimeMCPSession diff --git a/src/langbot/pkg/provider/tools/loaders/native.py b/src/langbot/pkg/provider/tools/loaders/native.py index 3913fbc5e..ca8defa6f 100644 --- a/src/langbot/pkg/provider/tools/loaders/native.py +++ b/src/langbot/pkg/provider/tools/loaders/native.py @@ -329,20 +329,11 @@ class NativeToolLoader(loader.ToolLoader): if not package_root: raise ValueError(f'Activated skill "{selected_skill_name}" has no package_root.') - # Pass only the logical name across the authenticated Core→Runtime - # boundary. In Cloud mode the shared Box Runtime resolves the - # Workspace-scoped package root and constructs the read-only mount; - # Core host paths are never accepted as mount authority. # Wrap command with Python venv bootstrap if the skill has a Python project. # The venv is created inside the skill's mount path. skill_mount = f'/workspace/.skills/{selected_skill_name}' python_project = selected_skill.get('python_project') is True - if 'python_project' not in selected_skill and bool( - getattr(self.ap.box_service, 'shares_filesystem_with_box', False) - ): - # Backward compatibility for a same-process OSS Runtime that - # predates trusted Box metadata. Never probe a path reported by - # an external Runtime from the Core filesystem. + if 'python_project' not in selected_skill: python_project = skill_loader.should_prepare_skill_python_env(package_root) if python_project: parameters = dict(parameters) @@ -358,12 +349,9 @@ class NativeToolLoader(loader.ToolLoader): result = await self.ap.box_service.execute_tool( parameters, query, - skill_name=selected_skill_name, + read_only_mounts=skill_loader.build_execution_mounts(self.ap, query), ) result = self._normalize_exec_result(result) - - if selected_skill is not None: - self._refresh_skill_from_disk(query, selected_skill) return result def _resolve_host_location( @@ -386,8 +374,7 @@ class NativeToolLoader(loader.ToolLoader): if selected_skill is not None: if not self._can_interpret_skill_host_paths(): raise ValueError( - 'Skill package paths are owned by the Box Runtime; ' - 'this operation requires a Runtime skill-file API.' + 'Secure Core host file operations are unavailable on this platform.' ) host_root = selected_skill.get('package_root') workspace_anchor = None @@ -426,11 +413,9 @@ class NativeToolLoader(loader.ToolLoader): return selected_skill, relative def _can_interpret_skill_host_paths(self) -> bool: - """Require an explicitly proven shared Core/Runtime filesystem view.""" + """Return whether Core can use its no-follow host file primitives.""" - return _SECURE_HOST_FILE_OPS_AVAILABLE and bool( - getattr(self.ap.box_service, 'shares_filesystem_with_box', False) - ) + return _SECURE_HOST_FILE_OPS_AVAILABLE def _should_use_box_workspace_files(self, selected_skill: dict | None) -> bool: if selected_skill is not None: @@ -1130,16 +1115,6 @@ else: skill_repository = getattr(self.ap, 'skill_repository', None) if skill_request is not None and skill_repository is not None: selected_skill, relative = skill_request - if self._can_interpret_skill_host_paths(): - host_location = self._resolve_skill_host_location(selected_skill, relative) - else: - host_location = None - if host_location is not None: - try: - return await asyncio.to_thread(self._read_host_location, host_location, parameters) - except FileNotFoundError: - pass - try: result = await skill_repository.read_skill_file( self._execution_context(query), diff --git a/src/langbot/pkg/provider/tools/loaders/skill.py b/src/langbot/pkg/provider/tools/loaders/skill.py index 82f93278d..fec3b6e00 100644 --- a/src/langbot/pkg/provider/tools/loaders/skill.py +++ b/src/langbot/pkg/provider/tools/loaders/skill.py @@ -1,10 +1,14 @@ from __future__ import annotations +import os import re import typing -from ....box import workspace as box_workspace from ....api.http.context import ExecutionContext +from ....utils.python_workspace import ( + should_prepare_python_env, + wrap_python_command_with_env, +) if typing.TYPE_CHECKING: from ....core import app @@ -57,6 +61,30 @@ def get_visible_skill(ap: app.Application, query: pipeline_query.Query, skill_na return get_visible_skills(ap, query).get(skill_name) +def build_execution_mounts(ap: app.Application, query: pipeline_query.Query) -> list[dict]: + """Translate visible Core-owned packages into generic read-only mounts.""" + + mounts: list[dict] = [] + for skill_name, skill_data in get_visible_skills(ap, query).items(): + package_root = str(skill_data.get('package_root', '') or '').strip() + if not package_root: + continue + if not os.path.isdir(package_root): + ap.logger.warning( + f'Skill "{skill_name}" package_root missing on the Core filesystem ' + f'({package_root}); skipping its execution mount. Reload the skill catalog.' + ) + continue + mounts.append( + { + 'host_path': package_root, + 'mount_path': get_virtual_skill_mount_path(skill_name), + 'mode': 'ro', + } + ) + return mounts + + def get_activated_skills(query: pipeline_query.Query) -> dict[str, dict]: if query.variables is None: return {} @@ -198,7 +226,7 @@ def build_skill_session_id(skill_data: dict, query: pipeline_query.Query) -> str def should_prepare_skill_python_env(package_root: str | None) -> bool: - return box_workspace.should_prepare_python_env(package_root) + return should_prepare_python_env(package_root) def wrap_skill_command_with_python_env( @@ -207,7 +235,7 @@ def wrap_skill_command_with_python_env( mount_path: str = '/workspace', state_path: str | None = None, ) -> str: - return box_workspace.wrap_python_command_with_env( + return wrap_python_command_with_env( command, mount_path=mount_path, state_path=state_path, diff --git a/src/langbot/pkg/skill/repository.py b/src/langbot/pkg/skill/repository.py index 2d04f7cf3..9f73ba6e9 100644 --- a/src/langbot/pkg/skill/repository.py +++ b/src/langbot/pkg/skill/repository.py @@ -21,6 +21,7 @@ class SkillRepository: self.ap = ap config = getattr(getattr(ap, 'instance_config', None), 'data', {}) or {} self._local_config = (config.get('box') or {}).get('local') or {} + self._skills_config = config.get('skills') or {} self._store = SkillStore(self._skills_root()) self._lock = asyncio.Lock() @@ -29,9 +30,15 @@ class SkillRepository: return os.path.realpath(os.path.abspath(os.path.expanduser(configured))) def _skills_root(self) -> str: - configured = str(self._local_config.get('skills_root') or 'skills').strip() - if not os.path.isabs(configured): - configured = os.path.join(self._host_root(), configured) + configured = str(self._skills_config.get('root') or '').strip() + if not configured: + # Online-upgrade bridge for installations whose persisted config + # predates the standalone Skill domain. + # TODO(next-major): remove box.local.skills_root fallback. + legacy = str(self._local_config.get('skills_root') or '').strip() + configured = legacy or 'skills' + if not os.path.isabs(configured): + configured = os.path.join(self._host_root(), configured) return os.path.realpath(os.path.abspath(os.path.expanduser(configured))) def _default_workspace(self) -> str: diff --git a/src/langbot/pkg/telemetry/heartbeat.py b/src/langbot/pkg/telemetry/heartbeat.py index 45dcbf268..69b73a97b 100644 --- a/src/langbot/pkg/telemetry/heartbeat.py +++ b/src/langbot/pkg/telemetry/heartbeat.py @@ -198,7 +198,7 @@ async def build_heartbeat_payload( except Exception: features['plugin_count'] = -1 - # Skill count (from Box runtime via skill manager) + # Skill count (from the Core SkillRepository cache) try: skill_mgr = getattr(ap, 'skill_mgr', None) if skill_mgr is not None: diff --git a/src/langbot/pkg/utils/python_workspace.py b/src/langbot/pkg/utils/python_workspace.py new file mode 100644 index 000000000..b21f7a694 --- /dev/null +++ b/src/langbot/pkg/utils/python_workspace.py @@ -0,0 +1,195 @@ +"""Python project detection and sandbox-local environment bootstrap helpers.""" + +from __future__ import annotations + +import os +import textwrap + + +PYTHON_MANIFEST_FILES = ( + 'requirements.txt', + 'pyproject.toml', + 'setup.py', + 'setup.cfg', +) + + +def normalize_host_path(path: str | None) -> str: + if path is None: + return '' + stripped = str(path).strip() + if not stripped: + return '' + return os.path.realpath(os.path.abspath(stripped)) + + +def list_python_manifest_files(host_path: str | None) -> list[str]: + normalized_root = normalize_host_path(host_path) + if not normalized_root: + return [] + return [ + filename + for filename in PYTHON_MANIFEST_FILES + if os.path.isfile(os.path.join(normalized_root, filename)) + ] + + +def should_prepare_python_env(host_path: str | None) -> bool: + normalized_root = normalize_host_path(host_path) + if not normalized_root: + return False + if os.path.isdir(os.path.join(normalized_root, '.venv')): + return True + return bool(list_python_manifest_files(normalized_root)) + + +def wrap_python_command_with_env( + command: str, + *, + mount_path: str = '/workspace', + state_path: str | None = None, +) -> str: + """Wrap a command with a reusable sandbox-local Python env bootstrap.""" + + writable_state_path = state_path or mount_path + bootstrap = textwrap.dedent( + f""" + set -e + + _LB_VENV_DIR="{writable_state_path}/.venv" + _LB_META_DIR="{writable_state_path}/.langbot" + _LB_META_FILE="$_LB_META_DIR/python-env.json" + _LB_LOCK_DIR="$_LB_META_DIR/python-env.lock" + _LB_TMP_DIR="{writable_state_path}/.tmp" + _LB_PIP_CACHE_DIR="{writable_state_path}/.cache/pip" + + mkdir -p "$_LB_META_DIR" "$_LB_TMP_DIR" "$_LB_PIP_CACHE_DIR" + _LB_SYSTEM_PYTHON="$(command -v python3 || command -v python || true)" + if [ -z "$_LB_SYSTEM_PYTHON" ]; then + echo "python3 or python is required to prepare the workspace Python environment" >&2 + exit 127 + fi + + export TMPDIR="$_LB_TMP_DIR" + export TEMP="$_LB_TMP_DIR" + export TMP="$_LB_TMP_DIR" + export PIP_CACHE_DIR="$_LB_PIP_CACHE_DIR" + + _lb_python_meta() {{ + "$_LB_SYSTEM_PYTHON" - <<'PY' + import hashlib + import json + import os + import sys + + root = "{mount_path}" + max_manifest_bytes = 10 * 1024 * 1024 + digest = hashlib.sha256() + manifest_files = [] + for rel in ("requirements.txt", "pyproject.toml", "setup.py", "setup.cfg"): + path = os.path.join(root, rel) + if not os.path.isfile(path): + continue + if os.path.getsize(path) > max_manifest_bytes: + raise RuntimeError( + f"Python project manifest exceeds {{max_manifest_bytes}} bytes: {{rel}}" + ) + manifest_files.append(rel) + with open(path, "rb") as handle: + digest.update(rel.encode("utf-8")) + digest.update(b"\0") + while chunk := handle.read(1024 * 1024): + digest.update(chunk) + digest.update(b"\0") + + print( + json.dumps( + {{ + "python_executable": sys.executable, + "python_version": list(sys.version_info[:3]), + "manifest_files": manifest_files, + "manifest_sha256": digest.hexdigest(), + }}, + sort_keys=True, + ) + ) + PY + }} + + _LB_CURRENT_META="$(_lb_python_meta)" + _LB_NEEDS_BOOTSTRAP=0 + + if [ ! -x "$_LB_VENV_DIR/bin/python" ]; then + _LB_NEEDS_BOOTSTRAP=1 + elif [ ! -f "$_LB_META_FILE" ]; then + _LB_NEEDS_BOOTSTRAP=1 + elif [ "$(cat "$_LB_META_FILE")" != "$_LB_CURRENT_META" ]; then + _LB_NEEDS_BOOTSTRAP=1 + fi + + if [ "$_LB_NEEDS_BOOTSTRAP" -eq 1 ]; then + _LB_LOCK_WAIT=0 + while ! mkdir "$_LB_LOCK_DIR" 2>/dev/null; do + if [ "$_LB_LOCK_WAIT" -ge 120 ]; then + _LB_LOCK_OWNER="$(cat "$_LB_LOCK_DIR/pid" 2>/dev/null || true)" + if [ -n "$_LB_LOCK_OWNER" ] && kill -0 "$_LB_LOCK_OWNER" 2>/dev/null; then + echo "Timed out waiting for active Python environment lock: $_LB_LOCK_DIR" >&2 + exit 1 + fi + echo "Timed out waiting for Python environment lock, clearing stale lock: $_LB_LOCK_DIR" >&2 + rm -rf "$_LB_LOCK_DIR" 2>/dev/null || true + if mkdir "$_LB_LOCK_DIR" 2>/dev/null; then + break + fi + echo "Timed out waiting for Python environment lock: $_LB_LOCK_DIR" >&2 + exit 1 + fi + sleep 1 + _LB_LOCK_WAIT=$((_LB_LOCK_WAIT + 1)) + done + printf '%s\n' "$$" > "$_LB_LOCK_DIR/pid" 2>/dev/null || true + + _lb_cleanup_lock() {{ + rm -rf "$_LB_LOCK_DIR" >/dev/null 2>&1 || true + }} + trap _lb_cleanup_lock EXIT INT TERM + + _LB_CURRENT_META="$(_lb_python_meta)" + _LB_NEEDS_BOOTSTRAP=0 + if [ ! -x "$_LB_VENV_DIR/bin/python" ]; then + _LB_NEEDS_BOOTSTRAP=1 + elif [ ! -f "$_LB_META_FILE" ]; then + _LB_NEEDS_BOOTSTRAP=1 + elif [ "$(cat "$_LB_META_FILE")" != "$_LB_CURRENT_META" ]; then + _LB_NEEDS_BOOTSTRAP=1 + fi + + if [ "$_LB_NEEDS_BOOTSTRAP" -eq 1 ]; then + rm -rf "$_LB_VENV_DIR" + "$_LB_SYSTEM_PYTHON" -m venv "$_LB_VENV_DIR" + . "$_LB_VENV_DIR/bin/activate" + python -m pip install --upgrade pip setuptools wheel + if [ -f "{mount_path}/requirements.txt" ]; then + python -m pip install -r "{mount_path}/requirements.txt" + elif [ -f "{mount_path}/pyproject.toml" ] || [ -f "{mount_path}/setup.py" ] || [ -f "{mount_path}/setup.cfg" ]; then + python -m pip install "{mount_path}" + fi + printf '%s' "$_LB_CURRENT_META" > "$_LB_META_FILE" + fi + fi + + export VIRTUAL_ENV="$_LB_VENV_DIR" + export PATH="$_LB_VENV_DIR/bin:$PATH" + {command} + """ + ).strip() + return bootstrap + '\n' + + +__all__ = [ + 'PYTHON_MANIFEST_FILES', + 'list_python_manifest_files', + 'normalize_host_path', + 'should_prepare_python_env', + 'wrap_python_command_with_env', +] diff --git a/src/langbot/templates/config.yaml b/src/langbot/templates/config.yaml index 797fadfac..d09943c57 100644 --- a/src/langbot/templates/config.yaml +++ b/src/langbot/templates/config.yaml @@ -323,6 +323,13 @@ monitoring: # Prevent one large Workspace backlog from monopolizing PostgreSQL. # Supports MONITORING__AUTO_CLEANUP__MAX_BATCHES_PER_TABLE_PER_RUN. max_batches_per_table_per_run: 4 +skills: + # Core-owned SkillStore. Skill discovery, activation, resources, CRUD and + # revisions remain available when Box is disabled. + # TODO(next-major): change the fresh-install default to './data/skills' + # after the online-upgrade window for the historical Box path closes. + root: './data/box/skills' + box: # Master switch for the Box sandbox runtime. When false, LangBot does NOT # attempt to connect to a remote Box runtime nor start a local stdio Box @@ -378,7 +385,6 @@ box: image: '' # Custom local sandbox image. Leave empty to use the profile default. host_root: './data/box' # Base host directory for local workspace mounts. Docker deployments should override this with an absolute host path. default_workspace: '' # Defaults to '/default'. Relative paths are resolved under host_root. - skills_root: 'skills' # Core-owned skill repository shared with Box for optional execution mounts. allowed_mount_roots: # Defaults to [''] when left empty. - './data/box' - '/tmp' diff --git a/tests/integration_tests/box/test_cloud_box_admission_integration.py b/tests/integration_tests/box/test_cloud_box_admission_integration.py index 96610d511..f38289789 100644 --- a/tests/integration_tests/box/test_cloud_box_admission_integration.py +++ b/tests/integration_tests/box/test_cloud_box_admission_integration.py @@ -29,6 +29,9 @@ from langbot.pkg.cloud.entitlements import ( EntitlementSnapshot, EntitlementUnavailableError, ) +from langbot.pkg.skill.manager import SkillManager +from langbot.pkg.skill.repository import SkillRepository +from langbot.pkg.provider.tools.loaders import skill as skill_loader pytestmark = pytest.mark.integration @@ -54,7 +57,7 @@ class _AdmissionBackend(BaseSandboxBackend): 'mount_isolation': True, 'network_isolation': True, 'hard_workspace_quota': True, - 'hard_skill_storage_quota': True, + 'hard_read_only_mount_quota': True, 'bounded_ephemeral_storage': True, 'inode_quota': True, } @@ -230,9 +233,18 @@ async def _stack(tmp_path): deployment=SimpleNamespace(multi_workspace_enabled=True), entitlement_resolver=EntitlementResolver('instance-a', entitlements), workspace_service=workspace_service, - instance_config=SimpleNamespace(data={'box': box_config, 'system': {'limitation': {}}}), + instance_config=SimpleNamespace( + data={ + 'skills': {'root': str(shared_root / 'skills')}, + 'box': box_config, + 'system': {'limitation': {}}, + } + ), ) + app.skill_repository = SkillRepository(app) + app.skill_mgr = SkillManager(app) service = BoxService(app, client=client) + app.box_service = service await service.initialize() return service, runtime, backend, entitlements, server_task, client_task @@ -312,7 +324,7 @@ async def test_two_workspaces_get_isolated_physical_sessions_and_paths(tmp_path) @pytest.mark.asyncio -async def test_cloud_skills_reject_host_paths_and_require_managed_entitlement(tmp_path): +async def test_cloud_core_skills_mount_generically_and_do_not_require_box_entitlement(tmp_path): service, runtime, backend, entitlements, server_task, client_task = await _stack(tmp_path) first = _context('workspace-a') second = _context('workspace-b') @@ -324,32 +336,35 @@ async def test_cloud_skills_reject_host_paths_and_require_managed_entitlement(tm managed=False, ) try: - private = await service.create_skill( + repository = service.ap.skill_repository + await repository.create_skill( second, { 'name': 'private', 'instructions': 'workspace-b secret', }, ) - own_skill = await service.create_skill( + own_skill = await repository.create_skill( first, { 'name': 'runner', 'instructions': 'Run scripts/main.py', }, ) - await service.write_skill_file(first, 'runner', 'scripts/main.py', "print('ok')") - await service.write_skill_file(first, 'runner', 'requirements.txt', 'requests==2.32.0\n') - refreshed_skill = await service.get_skill(first, 'runner') + await repository.write_skill_file(first, 'runner', 'scripts/main.py', "print('ok')") + await repository.write_skill_file(first, 'runner', 'requirements.txt', 'requests==2.32.0\n') + refreshed_skill = await repository.get_skill(first, 'runner') assert refreshed_skill is not None assert refreshed_skill['python_project'] is True + await service.ap.skill_mgr.reload_skills(first) + query = _query(first, 91) await service.execute_tool( { 'command': 'python /workspace/.skills/runner/scripts/main.py', 'workdir': '/workspace/.skills/runner', }, - _query(first, 91), - skill_name='runner', + query, + read_only_mounts=skill_loader.build_execution_mounts(service.ap, query), ) mounted_spec = backend.started_specs[-1] @@ -358,20 +373,14 @@ async def test_cloud_skills_reject_host_paths_and_require_managed_entitlement(tm assert mounted_spec.extra_mounts[0].mount_path == '/workspace/.skills/runner' assert mounted_spec.extra_mounts[0].mode.value == 'ro' - with pytest.raises(BoxAdmissionError, match='Scanning arbitrary host'): - await service.scan_skill_directory(first, private['package_root']) - with pytest.raises(BoxAdmissionError, match='package_root is runtime-owned'): - await service.create_skill( - first, - { - 'name': 'stolen', - 'package_root': private['package_root'], - }, - ) - - assert await service.get_skill(first, 'private') is None + assert await repository.get_skill(first, 'private') is None + await repository.create_skill( + ineligible, + {'name': 'docs-only', 'instructions': 'Read this without Box.'}, + ) + assert [skill['name'] for skill in await repository.list_skills(ineligible)] == ['docs-only'] with pytest.raises(EntitlementUnavailableError): - await service.list_skills(ineligible) + await service.execute_tool({'command': 'true'}, _query(ineligible, 92)) finally: server_task.cancel() client_task.cancel() diff --git a/tests/unit_tests/box/test_box_service.py b/tests/unit_tests/box/test_box_service.py index e9770ba9f..b9615a75d 100644 --- a/tests/unit_tests/box/test_box_service.py +++ b/tests/unit_tests/box/test_box_service.py @@ -41,6 +41,7 @@ from langbot_plugin.box.security import ( from langbot_plugin.entities.io.context import ActionContext from langbot.pkg.api.http.context import ExecutionContext from langbot.pkg.box.service import BoxService +from langbot.pkg.provider.tools.loaders import skill as skill_loader _UTC = dt.timezone.utc _CONTEXT = ExecutionContext( @@ -301,7 +302,7 @@ class TestSharesFilesystemWithBox: - stdio (local child process) → shared filesystem → True - WebSocket (Docker / sidecar / --standalone-box / remote) → separated → False - This drives whether LangBot validates Box-reported skill paths locally. + This drives whether LangBot can safely perform local workspace operations. Getting it wrong silently drops every skill in separated deployments. """ @@ -338,7 +339,7 @@ class TestSharesFilesystemWithBox: def test_false_when_client_injected_without_connector(self): # Injected client (no connector) → unknown topology → conservative False - # so LangBot never wrongly drops Box-reported skills. + # so LangBot does not assume a shared local filesystem. service = BoxService(make_app(Mock()), client=Mock(spec=BoxRuntimeClient)) assert service._runtime_connector is None @@ -552,7 +553,6 @@ async def test_box_service_reconnect_restores_workspace_and_runs_cleanup( monkeypatch: pytest.MonkeyPatch, ): app = make_app(Mock()) - app.skill_mgr = SimpleNamespace(reload_skills=AsyncMock()) service = BoxService(app, client=Mock(spec=BoxRuntimeClient)) connector = Mock() connector.reconnect = AsyncMock() @@ -565,16 +565,14 @@ async def test_box_service_reconnect_restores_workspace_and_runs_cleanup( connector.reconnect.assert_awaited_once() service._ensure_default_workspace.assert_called_once() service._purge_attachment_dirs.assert_awaited_once() - app.skill_mgr.reload_skills.assert_awaited_once() assert service.available is True @pytest.mark.asyncio -async def test_cloud_box_service_reconnect_does_not_reload_unscoped_skills( +async def test_cloud_box_service_reconnect_restores_runtime_only( monkeypatch: pytest.MonkeyPatch, ): app = make_app(Mock()) - app.skill_mgr = SimpleNamespace(reload_skills=AsyncMock()) service = BoxService(app, client=Mock(spec=BoxRuntimeClient)) service._cloud_managed = True connector = Mock() @@ -587,7 +585,6 @@ async def test_cloud_box_service_reconnect_does_not_reload_unscoped_skills( connector.reconnect.assert_awaited_once() service._verify_cloud_runtime.assert_awaited_once() - app.skill_mgr.reload_skills.assert_not_awaited() assert service.available is True @@ -1941,7 +1938,7 @@ def test_disconnect_callback_does_not_schedule_without_running_event_loop(): assert service._reconnecting is False -class TestBuildSkillExtraMounts: +class TestBuildSkillExecutionMounts: """Robustness of skill mount construction against a stale skill cache. The three sandbox backends behave inconsistently when a skill's @@ -1951,16 +1948,10 @@ class TestBuildSkillExtraMounts: the backend never sees a bad mount. """ - def _make_service(self, logger, skills, *, shares_filesystem=True): + def _make_app(self, logger, skills): app = make_app(logger) app.skill_mgr = SimpleNamespace(skills=skills, get_skills=Mock(return_value=skills)) - client = Mock(spec=BoxRuntimeClient) - service = BoxService(app, client=client) - # Tests construct BoxService with an injected client (no connector), so - # set the topology explicitly. Most cases exercise the shared-fs (local - # stdio) path where local package_root validation applies. - service._shares_filesystem_with_box_override = shares_filesystem - return service + return app def test_skips_skill_with_missing_package_root(self): logger = Mock() @@ -1969,10 +1960,10 @@ class TestBuildSkillExtraMounts: 'alive': {'name': 'alive', 'package_root': live_dir}, 'ghost': {'name': 'ghost', 'package_root': '/nonexistent/path/should/never/exist'}, } - service = self._make_service(logger, skills) + app = self._make_app(logger, skills) query = make_query() - mounts = service.build_skill_extra_mounts(query) + mounts = skill_loader.build_execution_mounts(app, query) assert mounts == [ { @@ -1987,27 +1978,19 @@ class TestBuildSkillExtraMounts: for call in logger.warning.call_args_list ) - def test_trusts_box_paths_when_filesystem_not_shared(self): - """In separated deployments (Docker Compose, k8s sidecar, - --standalone-box, remote endpoint) the Box runtime owns its own - filesystem. package_root values it reports are NOT resolvable on the - LangBot side, so LangBot must trust them rather than dropping every - skill via a local isdir() check.""" + def test_rejects_missing_core_paths_when_filesystem_not_shared(self): + """Core owns package paths even when Box is a separate process.""" logger = Mock() skills = { 'a': {'name': 'a', 'package_root': '/box/skills/a'}, 'b': {'name': 'b', 'package_root': '/box/skills/b'}, } - service = self._make_service(logger, skills, shares_filesystem=False) + app = self._make_app(logger, skills) - mounts = service.build_skill_extra_mounts(make_query()) + mounts = skill_loader.build_execution_mounts(app, make_query()) - assert mounts == [ - {'host_path': '/box/skills/a', 'mount_path': '/workspace/.skills/a', 'mode': 'ro'}, - {'host_path': '/box/skills/b', 'mount_path': '/workspace/.skills/b', 'mode': 'ro'}, - ] - # No skill is dropped, so no "missing" warning should be logged. - assert not any('package_root missing' in str(call.args[0]) for call in logger.warning.call_args_list) + assert mounts == [] + assert len(logger.warning.call_args_list) == 2 def test_skips_skill_with_empty_package_root(self): logger = Mock() @@ -2015,25 +1998,23 @@ class TestBuildSkillExtraMounts: 'no_root': {'name': 'no_root', 'package_root': ''}, 'whitespace': {'name': 'whitespace', 'package_root': ' '}, } - service = self._make_service(logger, skills) + app = self._make_app(logger, skills) - assert service.build_skill_extra_mounts(make_query()) == [] + assert skill_loader.build_execution_mounts(app, make_query()) == [] def test_empty_package_root_skipped_even_when_not_shared(self): """An empty package_root is always invalid regardless of topology.""" logger = Mock() skills = {'no_root': {'name': 'no_root', 'package_root': ''}} - service = self._make_service(logger, skills, shares_filesystem=False) + app = self._make_app(logger, skills) - assert service.build_skill_extra_mounts(make_query()) == [] + assert skill_loader.build_execution_mounts(app, make_query()) == [] def test_returns_empty_when_no_skill_manager(self): logger = Mock() app = make_app(logger) # no skill_mgr attribute - service = BoxService(app, client=Mock(spec=BoxRuntimeClient)) - - assert service.build_skill_extra_mounts(make_query()) == [] + assert skill_loader.build_execution_mounts(app, make_query()) == [] # ── Attachment passthrough (inbound / outbound) ───────────────────────────── diff --git a/tests/unit_tests/box/test_workspace.py b/tests/unit_tests/box/test_workspace.py index 48720a48a..f2a204413 100644 --- a/tests/unit_tests/box/test_workspace.py +++ b/tests/unit_tests/box/test_workspace.py @@ -13,8 +13,8 @@ from langbot.pkg.box.workspace import ( classify_python_workspace, infer_workspace_host_path, rewrite_mounted_path, - wrap_python_command_with_env, ) +from langbot.pkg.utils.python_workspace import wrap_python_command_with_env _CONTEXT = ExecutionContext( diff --git a/tests/unit_tests/provider/test_skill_tools.py b/tests/unit_tests/provider/test_skill_tools.py index 4e0355878..46db84f71 100644 --- a/tests/unit_tests/provider/test_skill_tools.py +++ b/tests/unit_tests/provider/test_skill_tools.py @@ -614,7 +614,7 @@ class TestNativeToolLoaderSkillPaths: ap.skill_repository.read_skill_file.assert_awaited_once_with(_CONTEXT, 'demo', 'SKILL.md') @pytest.mark.asyncio - async def test_external_runtime_rejects_skill_host_fallback_without_protocol_capability(self): + async def test_core_owned_skill_path_does_not_depend_on_runtime_topology(self): from langbot.pkg.provider.tools.loaders.native import NativeToolLoader from langbot.pkg.provider.tools.loaders.skill import PIPELINE_BOUND_SKILLS_KEY @@ -634,18 +634,20 @@ class TestNativeToolLoaderSkillPaths: variables={PIPELINE_BOUND_SKILLS_KEY: ['demo']}, ) - with pytest.raises(ValueError, match='owned by the Box Runtime'): - await loader.invoke_tool( - 'grep', - { - 'path': '/workspace/.skills/demo', - 'pattern': 'core-host-secret', - }, - query, - ) + result = await loader.invoke_tool( + 'grep', + { + 'path': '/workspace/.skills/demo', + 'pattern': 'core-host-secret', + }, + query, + ) + + assert result['ok'] is True + assert result['total'] == 1 @pytest.mark.asyncio - async def test_exec_in_activated_skill_mount_rewrites_command_and_refreshes(self): + async def test_exec_in_activated_skill_mount_rewrites_command_without_mutating_skill(self): from langbot.pkg.provider.tools.loaders.native import NativeToolLoader from langbot.pkg.provider.tools.loaders.skill import register_activated_skill @@ -656,11 +658,15 @@ class TestNativeToolLoaderSkillPaths: default_workspace=tmpdir, execute_tool=AsyncMock(return_value={'ok': True}), ) - ap.skill_mgr = SimpleNamespace(refresh_skill_from_disk=Mock()) + skill_data = _make_skill_data(name='demo', package_root=tmpdir) + ap.skill_mgr = _make_skill_manager( + {'demo': skill_data}, + refresh_skill_from_disk=Mock(), + ) loader = NativeToolLoader(ap) query = _make_query(query_id='q1', launcher_type='person', launcher_id='123') - register_activated_skill(query, _make_skill_data(name='demo', package_root=tmpdir)) + register_activated_skill(query, skill_data) result = await loader.invoke_tool( 'exec', @@ -675,8 +681,8 @@ class TestNativeToolLoaderSkillPaths: tool_parameters = ap.box_service.execute_tool.await_args.args[0] assert tool_parameters['command'] == 'python /workspace/.skills/demo/scripts/run.py' assert tool_parameters['workdir'] == '/workspace/.skills/demo' - assert ap.box_service.execute_tool.await_args.kwargs['skill_name'] == 'demo' - ap.skill_mgr.refresh_skill_from_disk.assert_called_once_with(_CONTEXT, 'demo') + assert 'skill_name' not in ap.box_service.execute_tool.await_args.kwargs + ap.skill_mgr.refresh_skill_from_disk.assert_not_called() @pytest.mark.asyncio async def test_external_runtime_python_skill_uses_trusted_metadata_and_writable_env(self): @@ -689,17 +695,18 @@ class TestNativeToolLoaderSkillPaths: shares_filesystem_with_box=False, execute_tool=AsyncMock(return_value={'ok': True}), ) - ap.skill_mgr = SimpleNamespace(refresh_skill_from_disk=Mock()) + skill_data = _make_skill_data( + name='demo', + package_root='/box-runtime/skills/tenants/workspace/demo', + python_project=True, + ) + ap.skill_mgr = _make_skill_manager( + {'demo': skill_data}, + refresh_skill_from_disk=Mock(), + ) loader = NativeToolLoader(ap) query = _make_query(query_id='q-external', launcher_type='person', launcher_id='123') - register_activated_skill( - query, - _make_skill_data( - name='demo', - package_root='/box-runtime/skills/tenants/workspace/demo', - python_project=True, - ), - ) + register_activated_skill(query, skill_data) result = await loader.invoke_tool( 'exec', @@ -716,7 +723,7 @@ class TestNativeToolLoaderSkillPaths: assert '_LB_VENV_DIR="/workspace/.skill-envs/demo/.venv"' in wrapped assert 'root = "/workspace/.skills/demo"' in wrapped assert '/box-runtime/skills/tenants/workspace/demo' not in wrapped - assert ap.box_service.execute_tool.await_args.kwargs['skill_name'] == 'demo' + assert 'skill_name' not in ap.box_service.execute_tool.await_args.kwargs @pytest.mark.asyncio async def test_write_requires_skill_activation(self): diff --git a/tests/unit_tests/test_skill_repository.py b/tests/unit_tests/test_skill_repository.py index 9fa3a7ded..783dfa09a 100644 --- a/tests/unit_tests/test_skill_repository.py +++ b/tests/unit_tests/test_skill_repository.py @@ -28,11 +28,11 @@ def _repository(tmp_path) -> SkillRepository: ), instance_config=SimpleNamespace( data={ + 'skills': {'root': str(tmp_path / 'skill-store')}, 'box': { 'enabled': False, 'local': { 'host_root': str(tmp_path / 'box'), - 'skills_root': 'skills', }, } } @@ -41,6 +41,32 @@ def _repository(tmp_path) -> SkillRepository: return SkillRepository(app) +def test_repository_prefers_standalone_skill_root(tmp_path): + repository = _repository(tmp_path) + + assert repository._store.root == str((tmp_path / 'skill-store').resolve()) + + +def test_repository_keeps_old_box_root_only_for_online_upgrade(tmp_path): + app = SimpleNamespace( + workspace_service=SimpleNamespace(get_execution_binding=_binding), + instance_config=SimpleNamespace( + data={ + 'box': { + 'local': { + 'host_root': str(tmp_path / 'box'), + 'skills_root': 'legacy-skills', + } + } + } + ), + ) + + repository = SkillRepository(app) + + assert repository._store.root == str((tmp_path / 'box' / 'legacy-skills').resolve()) + + @pytest.mark.asyncio async def test_repository_crud_and_reads_do_not_require_box(tmp_path): repository = _repository(tmp_path)