From 9d175c61108748921e4ebb2caa35640a7eb8df97 Mon Sep 17 00:00:00 2001 From: limityan Date: Fri, 31 Jul 2026 01:08:42 +0800 Subject: [PATCH 1/2] docs: design external command and skill compatibility --- .../external-ai-work-sources-design.md | 10 +++---- .../opencode-config-assets-adapter-design.md | 30 ++++++++++++++----- .../opencode-extension-compatibility.md | 4 +-- .../adapters/opencode-adapter/AGENTS.md | 13 +++++--- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/docs/architecture/extensions/external-ai-work-sources-design.md b/docs/architecture/extensions/external-ai-work-sources-design.md index d42dddf10..667843c3d 100644 --- a/docs/architecture/extensions/external-ai-work-sources-design.md +++ b/docs/architecture/extensions/external-ai-work-sources-design.md @@ -274,11 +274,11 @@ OpenCode Subagent 属于 L2：adapter 只读取声明，不执行外部代码； | 能力 | OpenCode | Claude Code | Codex | 当前边界 | |---|---|---|---|---| -| Prompt Command | JSON/JSONC、Markdown 的 prompt-only 安全子集 | legacy `commands/**/*.md` 的 prompt-only 安全子集；Skills 仍由 Skill 归属模块处理 | 没有稳定、独立于 Skills 的声明式 Command 来源，因此不伪造 provider | `$ARGUMENTS`/位置参数可展开；shell、文件动态引用、指定 Agent/模型等未接通语义整体受限，不做部分执行。 | +| Prompt Command | JSON/JSONC、Markdown 的 prompt-only 与静态本地文本文件子集 | legacy `commands/**/*.md` 的同一静态本地文本文件子集；Skills 仍由 Skill 归属模块处理 | 没有稳定、独立于 Skills 的声明式 Command 来源，因此不伪造 provider | `$ARGUMENTS`/位置参数及模板内 workspace 相对 UTF-8 `@file` 可展开；shell、动态/绝对/越界文件、指定 Agent/模型等整体受限。 | | Subagent | 用户/项目声明的安全子集 | 用户/项目 `agents/**/*.md` 的安全子集 | 用户/项目 `[agents]`、角色文件与安全配置层子集 | prompt、描述、精确模型和可表达工具请求进入既有归属模块；权限、私有 MCP/Hook、推理/并发等没有对应实现的字段会阻止激活。 | | MCP | 用户/显式目录/项目配置的安全子集 | user/project/local 原生层的安全子集 | 用户与项目 `config.toml` 原生层的安全子集 | 支持可表达的 stdio 与 HTTPS Streamable HTTP；发现不启动 Server，首次激活继续经 BitFun MCP 审批。OAuth、remote executor、per-tool policy 等不完整语义明确降级。 | | Standalone Tool | 已有单文件 JavaScript 子集 | 无稳定的 runtime-free standalone Tool 来源 | 无稳定的 runtime-free standalone Tool 来源 | TypeScript、package/plugin Tool 与动态工具注册依赖独立 Plugin Host，不在声明式 adapter 中猜测。 | -| Skill | 由现有 Skill 加载模块发现 `.opencode` 等标准根 | 由现有 Skill 加载模块发现 `.claude` 标准根；目录名是调用身份，描述可回退正文首段，`when_to_use` 合入索引，声明参数可做纯文本命名展开 | 由现有 Skill 加载模块发现 `.codex`、`.agents` 标准根；`.codex` 缺少 `name` 时回退目录名 | Skill 的加载、覆盖、模式开关与执行仍由同一个 Skill 模块负责，不复制进外部来源管理模块；未接通的运行时字段整体拒绝。 | +| Skill | 由现有 Skill 加载模块发现 `.opencode` 标准根及 OpenCode 本地配置根 | 由现有 Skill 加载模块发现 `.claude` 标准根；目录名是调用身份，描述可回退正文首段，`when_to_use` 合入索引，声明参数可做纯文本命名展开 | 由现有 Skill 加载模块发现 `.codex`、`.agents` 标准根；`.codex` 缺少 `name` 时回退目录名 | OpenCode V1 `skills.paths`/当前本地字符串数组只经 `bitfun-core/external_sources` 组合边界投影根目录，递归、加载、覆盖、模式开关与执行仍由同一个 Skill 模块负责；URL 不加载。 | | Hook | 静态目录 | 脱敏目录；同步 command 子集可审阅导入 | 脱敏目录；同步 command 子集可审阅导入 | 仅复制到私有原生快照并由 `AgentHookEngine` 执行；OpenCode、非 command、异步、未知或依赖未观察激活语义的 handler 不导入。 | 生态原生语义由各 adapter 以契约测试固定，不抽象成全局优先级： @@ -286,7 +286,7 @@ OpenCode Subagent 属于 L2：adapter 只读取声明，不执行外部代码； - Claude legacy Command 扫描用户与项目 `.claude/commands/**/*.md`，保留 `frontend/component` 到 `/frontend:component` 的原生命名空间；同层重名无效，遵循 Claude Code 当前“personal 覆盖 project”的 Skill/legacy Command 规则，同名 Skill 仅通过有界名称索引遮蔽 Command。 - 只展开 `$ARGUMENTS`、`$ARGUMENTS[N]` 和 `$N` 纯文本参数；shell、文件引用和改变 Agent、模型、工具或 Hook 的字段整体阻止激活。 + 展开 `$ARGUMENTS`、`$ARGUMENTS[N]` 和 `$N` 纯文本参数，并允许原模板中的静态 workspace 相对 `@file`；shell、动态/绝对/越界文件引用和改变 Agent、模型、工具或 Hook 的字段整体阻止激活。 - Skill Registry 继续拥有所有根的发现、覆盖、显式加载与刷新，只用既有稳定 source slot 在内部选择格式方言，不向用户 暴露主选择器，也不按路径字符串临时猜测。`.claude` Skill 的调用名固定为目录名；`description` 缺失时取正文首个 非空段落，并与可选 `when_to_use` 合并为最多 1536 个 Unicode 字符的模型索引说明。`arguments` 可为以空白分隔的名称 @@ -480,8 +480,8 @@ Command；明确缺失且未被标记失败的 Command 是稳定删除。产品 2. 发现 OpenCode 当前支持的用户全局和项目 Command 来源，建立来源限定身份、生态内覆盖关系和聚合清单； OpenCode 自身定义的项目/用户优先级仍由 adapter 解释，跨 provider 或与 BitFun 本地 Command 的同名冲突进入待选择状态。 3. 支持 `$ARGUMENTS` 与位置参数的 prompt-only 命令在用户显式选择或输入时展开并提交；发现本身不向会话发送内容。 -4. 含 `!shell`、`@file`、`{env:...}`、`{file:...}`、`agent`、`model`、`variant` 或 `subtask` 等未接通语义的命令标记为“部分受限”，不做 - 静默忽略后的部分执行。 +4. 模板内静态 workspace 相对 `@file` 经有界 UTF-8 读取后原子装配；含 `!shell`、动态/绝对/越界文件引用、`{env:...}`、 + `{file:...}`、`agent`、`model`、`variant` 或 `subtask` 等未接通语义的命令标记为“部分受限”，不做静默忽略后的部分执行。 5. Desktop 提供统一来源状态、刷新、按执行域抑制/恢复和冲突候选选择；首次 provider 扫描完成前显示中性检查状态， 不把暂时空目录误报为最终空结果；已经选择且内容摘要未变化的冲突退出待处理区。交互式 TUI（ChatMode）使用同一目录列出和执行 Command；跨 provider 候选由现有命令菜单按来源、使用范围和兼容状态展示，并通过内部稳定 candidate ID 选择，同次选择也解析本地同名冲突。发现或确认不阻塞 diff --git a/docs/architecture/extensions/opencode-config-assets-adapter-design.md b/docs/architecture/extensions/opencode-config-assets-adapter-design.md index a92e55dd7..ac62e3a60 100644 --- a/docs/architecture/extensions/opencode-config-assets-adapter-design.md +++ b/docs/architecture/extensions/opencode-config-assets-adapter-design.md @@ -200,9 +200,9 @@ OpenCode adapter 在来源发现、解析和审批前不 import module、不读 |---|---|---|---|---| | Rules / Instructions | 项目/全局 `AGENTS.md`、Claude fallback、`instructions` glob、本地文件、远程 URL | Workspace Instructions 归属模块保存有序来源引用 | 当前实现项目根与 `.opencode` 配置中的本地精确文件/glob；全局与远程 URL 仍是目标 | 无效 JSONC 或 glob 只排除对应配置项；文件 I/O 失败时当前构建不缓存并在下一条消息重试。 | | Agents / Modes | JSON、Markdown、description、mode、prompt、model、variant、temperature、top_p、steps、deprecated `maxSteps`、deprecated `tools`、permission、disable、options、hidden、color | Agent 归属模块创建兼容定义和使用范围视图 | 当前支持 Subagent 安全子集；首次按行为、来源、模型和工具范围确认，fresh single-run 调用 | primary/mode、permission、variant/options、采样、steps 与续接保持诊断或阻断，不影响其他 Agent。 | -| Skills | `.opencode/.claude/.agents` 项目与用户根、`SKILL.md`、`skills.paths/urls` | Skill 归属模块复用按需加载并补齐规则顺序 | 说明和索引按需加载；URL、脚本或外部依赖按 L2 确认 | URL 或可执行资源失败只降级对应 Skill。 | +| Skills | `.opencode/.claude/.agents` 项目与用户根、`SKILL.md`、`skills.paths/urls` | OpenCode adapter 只由 `bitfun-core/external_sources` 组合并投影有序本地配置根；Skill 归属模块负责有界递归、解析、覆盖与按需加载 | 标准根及 V1 `skills.paths`/当前本地字符串数组可用；项目配置限项目根，用户配置限项目根或用户目录；配置根最多 64 个、每根 512 个 Skill、单文件 256 KiB、可选策略 64 KiB，实际加载再次执行有界非链接读取；配置根在同 scope 覆盖标准 OpenCode 根，但不重排更早的 BitFun/Claude/Codex/Cursor 来源 | URL、下载/缓存、脚本与外部依赖不加载；无效根不影响标准 Skill。 | | References | `references` / 旧 `reference`，本地 path 或 Git repository/branch/description/hidden | **基础能力缺失**：先补 Workspace Reference 的异步准备与 `@alias` 消费接口 | 本地引用保留相对来源；Git 拉取按 L2 确认并保留缓存/隐藏语义 | 拉取失败不阻止项目，外部目录仍遵守工具权限。 | -| Commands | JSON/JSONC、Markdown、`$ARGUMENTS`、位置参数、`@file`、`!shell`、agent/model/variant/subtask | Prompt Command 专属契约；OpenCode adapter 保留发现、覆盖、解析和参数展开语义，交互式 TUI（ChatMode）只消费中立定义与展开结果 | 当前支持 prompt-only 模板，用户显式选择或输入即确认本次发送；未接通的文件、shell、agent/model/variant/subtask 标为部分受限且不做部分执行 | 已知命令文件无效只回退该命令；稳定删除撤下新调用；目录枚举未知时回退对应目录来源，不能把未知当空目录。 | +| Commands | JSON/JSONC、Markdown、`$ARGUMENTS`、位置参数、`@file`、`!shell`、agent/model/variant/subtask | Prompt Command 专属契约；adapter 提取静态文件引用，Product Assembly 经共享本地文本服务完成有界装配 | prompt-only 与静态 workspace 相对 UTF-8 `@file` 可发送；动态/绝对/越界文件、shell、agent/model/variant/subtask 整体受限 | 任一文件失败则本次调用原子失败；最多 8 文件、单文件 64 KiB、文件总量 128 KiB、最终命令 1 MiB。 | | MCP | local 的 command/environment/cwd/timeout，remote 的 URL/headers/oauth/timeout，Agent 选择 | MCP 归属模块创建兼容配置视图 | 当前支持 local stdio 和 HTTPS remote 的静态发现、首次/行为变化审批、冲突选择与 workspace 隔离的运行期接纳；C0a 快照导入只复制无 env/cwd 的 local command/args 或无 header/query/fragment 的 HTTPS remote，并保持 disabled | `{env:NAME}` 当前只允许用于运行期兼容来源的 environment/Header 值，不进入 C0a 快照；SSE、OpenCode OAuth client 配置、完整 timeout/Agent 范围与 Remote 执行域保持明确不支持；凭据或网络失败只影响单个 Server。 | | LSP | command、extensions、env、initialization | LSP 归属模块注册兼容实例 | 首次确认外部进程和使用范围后按文件类型启动 | 自定义 Server 缺少 extensions 或启动失败时只禁用该项。 | | Formatters | command、environment、extensions、`$FILE` | **基础能力缺失**：先补文件写入后的 Formatter 执行消费点，再做格式转换 | 首次确认命令后执行匹配 Formatter | 超时后标记未格式化，文件写入结果保留。 | @@ -258,12 +258,28 @@ OpenCode 会话内核、permission DSL 或 package plugin。Desktop/TUI 摘要 正文，静态 system prompt 也不因该适配而改写。来源 `description` 只进入审批和管理界面；已批准 Agent 进入现有 `` 动态视图时使用 BitFun 生成的稳定摘要，避免只改目录文案就绕过行为重批并改变模型上下文。 +OpenCode 本地配置 Skill 根不是新的外部资产生命周期。Adapter 按已实现的本地配置来源顺序累加每份有效文档中的 +V1 `skills.paths` 和当前 `skills: string[]`；字段类型错误只拒绝该文档的 Skill 根贡献，不影响同文档 Command 等其他能力。 +`skills.urls` 与 HTTP(S) 项不获取；相对路径只从当前本地 workspace 解析，项目配置根必须留在项目内，用户配置根必须 +留在项目或当前用户目录内，远程 workspace 的相对根不回退到本机解释。 + +`bitfun-core/external_sources` 是唯一构造 adapter 并投影本地根事实的组合边界，Skill Registry 不 import 生态 adapter，继续 +拥有递归发现、解析、覆盖、模式开关与实际加载。配置项最多保留 64 个；每根扫描深度 16、最多访问 4096 个条目和 +2048 个目录、接纳 512 个 `SKILL.md`，单个 Skill 限制 256 KiB，可选 `agents/openai.yaml` 限制 64 KiB。扫描和实际加载都 +拒绝符号链接/reparse point；加载时重新校验规范化根及其稳定 source slot，防止目录整体替换改变已发现来源身份。同 scope +内配置根位于标准 OpenCode 根之前，较后的不同配置根覆盖同名 Skill，但不重排更早的 BitFun/Claude/Codex/Cursor 来源。 + ### 5.3 Commands -当前 Prompt Command 子集只展开 `$ARGUMENTS` 与 `$1`、`$2` 等位置参数。OpenCode adapter 负责参数拆分、替换顺序和未使用参数追加， -Prompt Command 归属模块只接收最终可发送文本；产品 core 不按生态 ID 解释模板。包含 `@file`、`!shell`、 -`{env:...}`、`{file:...}`、agent/model/variant/subtask 的命令仍进入目录，但整体标为“部分受限”，不能解析凭据或 -删除不支持的部分后继续发送。 +当前 Prompt Command 子集展开 `$ARGUMENTS` 与 `$1`、`$2` 等位置参数，并支持模板中可静态确认的 workspace 相对 UTF-8 +`@file`。OpenCode adapter 只从原模板提取引用，不扫描用户参数；Product Assembly 在 stale/冲突校验后通过共享本地文本服务 +原子读取并追加内容。动态占位、绝对/`~`/URL/越界路径仍进入目录但整体受限。包含 `!shell`、`{env:...}`、`{file:...}`、 +agent/model/variant/subtask 的命令同样保持受限，不能删除不支持的部分后继续发送。 + +每次调用最多接纳 8 个不同文件，单文件 64 KiB、文件总量 128 KiB、最终命令 1 MiB。共享服务对每级路径执行 +workspace 规范化包含校验并拒绝符号链接/reparse point；任一引用缺失、越界、超限或不是 UTF-8 时整次调用失败，不返回 +部分装配结果。Adapter 在分配参数展开结果前先执行保守上界检查，Product Assembly 再按实际最终长度复核。规范化后再打开 +仍存在同一用户并发替换文件的本地 TOCTOU 窗口；读取始终受大小限制，且不会因此放宽到任意宿主机绝对路径。 Markdown front matter 的 `description`、`agent`、`model`、`variant`、`subtask` 按当前 OpenCode schema 校验； 已知字段类型错误使该命令不可用，不能当作缺省值继续执行。初次 YAML 解析失败时，adapter 按 OpenCode 当前规则将 @@ -274,7 +290,7 @@ provider 的模板正文总量限制为 8 MiB；超过限制进入明确诊断 正文不进入 IPC。执行前以来源限定命令 ID 和命令内容版本校验当前菜单项；若文件在菜单展示后更新，旧菜单项必须返回 stale selection 并等待重新选择，不能直接执行刚刷新的新内容。 -后续阶段接通文件引用和 shell 输出时仍按 OpenCode 顺序展开。`!shell` 必须进入脚本执行域，不另建绕过可靠性控制 +后续阶段接通 shell 输出时仍按 OpenCode 顺序展开。`!shell` 必须进入脚本执行域，不另建绕过可靠性控制 的同步 shell 路径；展开有期限、取消和输出大小限制，大输出保存后只把引用交给命令模板。 OpenCode 生态内部仍按其规则覆盖同名内置命令，但跨独立 provider 或与 BitFun 本地命令同名时不得静默覆盖。 diff --git a/docs/architecture/extensions/opencode-extension-compatibility.md b/docs/architecture/extensions/opencode-extension-compatibility.md index 4d8b491ba..f589d475a 100644 --- a/docs/architecture/extensions/opencode-extension-compatibility.md +++ b/docs/architecture/extensions/opencode-extension-compatibility.md @@ -108,9 +108,9 @@ OpenCode，和 OpenCode 配置/插件进入 BitFun 是两个独立验收方向 | 独立 `tui.json/jsonc` | 融合现有能力 + 转换参数 | 未实现 | 可完整适配 | OC-R1 | 按 global、`OPENCODE_TUI_CONFIG`、project、`.opencode` 独立顺序加载，不能复用主配置优先级 | [TUI 来源](opencode-config-assets-adapter-design.md#32-tui-独立来源顺序) | | Rules / Instructions | 转换参数 | 部分实现：项目本地文件与 glob | 可完整适配 | OC-R1 | 当前由 Workspace Instructions owner 读取项目 `instructions` 的精确文件和 glob；全局、远程 URL、完整来源合并与变化监听仍未实现 | [声明式资产](opencode-config-assets-adapter-design.md#5-声明式资产映射) | | Agents / Modes | 融合现有能力 + 转换参数 | 部分实现：Subagent 安全子集 | 可主要适配 | OC-R1 | 已支持全局/项目 Markdown、JSON/JSONC 的 subagent/all、prompt、description、disable/hidden、精确模型与工具映射，并接入审批、冲突、更新、撤下和 fresh single-run Task；primary/mode、permission、variant/options、采样与续接明确降级 | [Agents 与 Skills](opencode-config-assets-adapter-design.md#52-agentsmodes-与-skills) | -| Skills | 转换参数 | 部分实现：现有 Registry 发现与来源/覆盖展示 | 可完整适配 | OC-R2 | 已沿用现有根顺序发现 `.opencode` 等用户/项目 Skill，并在 GUI/TUI 展示来源、使用范围和覆盖结果；尚未复现 OpenCode 按需加载、完整配置合并及 allow/deny/ask 顺序，也未并入外部来源策略模块 | [Agents 与 Skills](opencode-config-assets-adapter-design.md#52-agentsmodes-与-skills) | +| Skills | 转换参数 | 部分实现：标准根与本地配置根 | 可完整适配 | OC-R2 | 现有 Registry 除标准用户/项目根外，也通过 `bitfun-core/external_sources` 组合边界按 OpenCode 配置来源顺序累加 V1 `skills.paths` 与当前迁移后的本地字符串数组；仅接受项目根/用户目录内的本地目录并做有界递归发现。同 scope 配置根覆盖标准 OpenCode 根，但不重排更早的 BitFun/Claude/Codex/Cursor 来源。URL、下载/缓存、完整 allow/deny/ask 顺序及外部来源策略仍未实现 | [Agents 与 Skills](opencode-config-assets-adapter-design.md#52-agentsmodes-与-skills) | | References | 补基础能力 + 转换参数 | 未实现 | 可主要适配 | OC-R2 | R1 解析；R2 支持本地目录和 Git repository/branch/description/hidden，异步准备并接入 `@alias` | [声明式资产](opencode-config-assets-adapter-design.md#5-声明式资产映射) | -| Commands | 补扩展接口 + 转换参数 | 部分实现：prompt-only 子集 | 可完整适配 | OC-R2 | 已支持全局/项目 JSON、JSONC、Markdown 命令、`$ARGUMENTS` 与位置参数，并接入动态命令目录、运行时刷新和显式冲突选择；`@file`、shell、Agent/model/variant/subtask 保持受限且不做部分执行 | [Commands](opencode-config-assets-adapter-design.md#53-commands) | +| Commands | 补扩展接口 + 转换参数 | 部分实现：prompt-only + 本地文本文件子集 | 可完整适配 | OC-R2 | 已支持全局/项目 JSON、JSONC、Markdown 命令、参数展开、动态目录、刷新和显式冲突选择；模板中的静态 workspace 相对 `@file` 可在调用时有界读取，动态/绝对/越界文件引用、shell、Agent/model/variant/subtask 保持受限且不做部分执行 | [Commands](opencode-config-assets-adapter-design.md#53-commands) | | Models / Providers 配置 | 融合现有能力 | 未实现 | 可主要适配 | OC-R1 | 静态字段进入模型归属模块；动态模型、鉴权和请求头交给插件运行时 | [声明式资产](opencode-config-assets-adapter-design.md#5-声明式资产映射) | | MCP | 转换参数 | 部分实现：local stdio 与 HTTPS remote | 可完整适配 | OC-R2 | 已接入发现、审批、冲突、workspace 隔离、更新和启动反馈；SSE、OAuth、完整 timeout/Agent 范围仍不支持；Remote 不回退本机实例 | [MCP、LSP 与 Formatter](opencode-config-assets-adapter-design.md#54-mcplsp-与-formatter) | | LSP | 转换参数 | 未实现 | 可完整适配 | OC-R2 | R1 解析；R2 转换 command、extensions、env 和 initialization 并由 LSP 归属模块启动 | [MCP、LSP 与 Formatter](opencode-config-assets-adapter-design.md#54-mcplsp-与-formatter) | diff --git a/src/crates/adapters/opencode-adapter/AGENTS.md b/src/crates/adapters/opencode-adapter/AGENTS.md index 0bbd344bb..e41caafd9 100644 --- a/src/crates/adapters/opencode-adapter/AGENTS.md +++ b/src/crates/adapters/opencode-adapter/AGENTS.md @@ -4,7 +4,8 @@ The current crate owns the static OpenCode source preview used by the existing managed-package path, the OpenCode-specific implementations of command, -standalone-tool, subagent, and MCP provider contracts, and runtime-free mapping +standalone-tool, subagent, and MCP provider contracts, the bounded projection of +configured local Skill roots, and runtime-free mapping of caller-normalized tool Hook descriptors. It preserves OpenCode source discovery, precedence, formats, argument expansion, and versioned compatibility semantics. Shared source catalog, lifecycle coordination, file-watch implementation, @@ -16,8 +17,9 @@ Product-source boundary: - The current `load_opencode_package_adapter` entry remains static-preview only until OC-R1/OC-R2 replace its production role. Do not extend this P0 entry into another managed OpenCode package format. -- Standard OpenCode Command, standalone Tool, and Subagent config and directories - are current read-only live sources. Full plugin directories and package specs +- Standard OpenCode Command, standalone Tool, Subagent, and configured local + Skill paths are current read-only live sources. Configured Skill URLs remain + unsupported and must never be fetched. Full plugin directories and package specs remain target work rather than executable production sources. Source files need no BitFun import. Low-risk declarative results follow the user's auto-apply/ask preference; executable sources require a source, plugin, @@ -86,7 +88,10 @@ Product-source boundary: diagnostics, while event payload types must not be treated as Hook properties. The adapter must not load handlers, dispatch Hooks, or imply executable support. - The reviewed product assembly entrypoint selects and constructs the compiled - OpenCode adapter/provider and injects it into PluginRuntimeClient. It does not + OpenCode adapter/provider. External-source providers and configured Skill-root + facts are projected through `bitfun-core/external_sources`; managed-package + bindings are injected into PluginRuntimeClient. Product consumers do not + import the adapter directly. The composition layer does not discover dynamic sources, prepare dependencies, or import plugin modules. - Product Assembly may consume this crate only from reviewed composition modules such as `bitfun-core/plugin_runtime` or `bitfun-core/external_sources`; boundary From 2166d5cf4ee9f8cfc111638a8c3fdaacdc8e994c Mon Sep 17 00:00:00 2001 From: limityan Date: Fri, 31 Jul 2026 02:36:36 +0800 Subject: [PATCH 2/2] feat: support local command files and OpenCode skill roots --- Cargo.lock | 1 + .../rules/source/forbidden-rules.mjs | 1 + .../rules/source/public-api-rules.mjs | 13 + scripts/core-boundaries/self-test.mjs | 4 +- .../claude-code-adapter/src/command_source.rs | 70 +- .../tests/command_source.rs | 86 +- .../opencode-adapter/src/command_source.rs | 102 ++- .../adapters/opencode-adapter/src/lib.rs | 4 + .../opencode-adapter/src/skill_source.rs | 189 +++++ .../tests/opencode_command_adapter.rs | 98 ++- .../tests/opencode_skill_roots.rs | 202 +++++ .../adapters/static-hook-support/Cargo.toml | 1 + .../adapters/static-hook-support/src/lib.rs | 157 +--- .../tools/implementations/skills/registry.rs | 756 +++++++++++++++++- .../assembly/core/src/external_sources.rs | 331 +++++++- .../assembly/external-sources/src/lib.rs | 10 +- .../tests/coordinator_contracts.rs | 14 +- .../product-domains/src/external_sources.rs | 10 +- .../tests/external_source_contracts.rs | 25 +- .../services/services-core/src/bounded_fs.rs | 170 ++++ src/crates/services/services-core/src/lib.rs | 2 + .../services/services-core/src/markdown.rs | 16 + .../services-core/src/workspace_text.rs | 272 +++++++ .../tests/markdown_owner_contracts.rs | 10 +- 24 files changed, 2310 insertions(+), 234 deletions(-) create mode 100644 src/crates/adapters/opencode-adapter/src/skill_source.rs create mode 100644 src/crates/adapters/opencode-adapter/tests/opencode_skill_roots.rs create mode 100644 src/crates/services/services-core/src/bounded_fs.rs create mode 100644 src/crates/services/services-core/src/workspace_text.rs diff --git a/Cargo.lock b/Cargo.lock index 7ae90427e..ee6a9f489 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1526,6 +1526,7 @@ name = "bitfun-static-hook-support" version = "0.2.14" dependencies = [ "bitfun-product-domains", + "bitfun-services-core", "hex", "serde_json", "sha2", diff --git a/scripts/core-boundaries/rules/source/forbidden-rules.mjs b/scripts/core-boundaries/rules/source/forbidden-rules.mjs index e5f61932a..b7f8287cc 100644 --- a/scripts/core-boundaries/rules/source/forbidden-rules.mjs +++ b/scripts/core-boundaries/rules/source/forbidden-rules.mjs @@ -4118,6 +4118,7 @@ export const forbiddenContentUnderRules = [ allowPaths: [ 'src/crates/adapters/opencode-adapter/tests/opencode_source_adapter.rs', 'src/crates/adapters/opencode-adapter/tests/opencode_command_adapter.rs', + 'src/crates/adapters/opencode-adapter/tests/opencode_skill_roots.rs', 'src/crates/adapters/opencode-adapter/tests/tool_source_contracts.rs', 'src/crates/adapters/opencode-adapter/tests/opencode_subagent_adapter.rs', 'src/crates/adapters/opencode-adapter/tests/opencode_mcp_adapter.rs', diff --git a/scripts/core-boundaries/rules/source/public-api-rules.mjs b/scripts/core-boundaries/rules/source/public-api-rules.mjs index c09927278..670e39dd3 100644 --- a/scripts/core-boundaries/rules/source/public-api-rules.mjs +++ b/scripts/core-boundaries/rules/source/public-api-rules.mjs @@ -211,6 +211,18 @@ export const opencodeAdapterPublicApiEntries = [ 'OpenCodeCommandProviderOptions', 'OpenCode command adapter fixture tests and explicit environment injection', ), + opencodeAdapterEntry( + 'OpenCodeConfiguredSkillRoot', + 'bitfun-core external source composition root and OpenCode configured Skill fixtures', + ), + opencodeAdapterEntry( + 'OpenCodeSkillRootProvider', + 'bitfun-core external source composition root and OpenCode configured Skill fixtures', + ), + opencodeAdapterEntry( + 'OpenCodeSkillRootProviderOptions', + 'OpenCode configured Skill fixture tests and explicit environment injection', + ), opencodeAdapterEntry( 'OpenCodeToolProvider', 'bitfun-core external source composition root and OpenCode standalone-tool adapter tests', @@ -584,6 +596,7 @@ export const externalSourceContractPublicApiEntries = [ 'PromptCommandAvailability', 'PromptCommandDefinition', 'ExpandedPromptCommand', + 'PromptCommandExpansion', 'PromptCommandProviderIdentity', 'PromptCommandProviderSnapshot', 'ExternalSourceContext', diff --git a/scripts/core-boundaries/self-test.mjs b/scripts/core-boundaries/self-test.mjs index 356439d14..2a782878f 100644 --- a/scripts/core-boundaries/self-test.mjs +++ b/scripts/core-boundaries/self-test.mjs @@ -1034,10 +1034,10 @@ export function runManifestParserSelfTest({ ).map((entry) => entry.symbol); if ( opencodeAdapterPublicApiSymbols.join(',') !== - 'load_opencode_package_adapter,OpenCodeCommandProvider,OpenCodeCommandProviderOptions,OpenCodeToolProvider,OpenCodeToolProviderOptions,OpenCodeSubagentProvider,OpenCodeSubagentProviderOptions,OpenCodeMcpProvider,OpenCodeMcpProviderOptions,OpenCodeHookProvider,OpenCodeHookProviderOptions' + 'load_opencode_package_adapter,OpenCodeCommandProvider,OpenCodeCommandProviderOptions,OpenCodeConfiguredSkillRoot,OpenCodeSkillRootProvider,OpenCodeSkillRootProviderOptions,OpenCodeToolProvider,OpenCodeToolProviderOptions,OpenCodeSubagentProvider,OpenCodeSubagentProviderOptions,OpenCodeMcpProvider,OpenCodeMcpProviderOptions,OpenCodeHookProvider,OpenCodeHookProviderOptions' ) { throw new Error( - 'OpenCode adapter public API budget must stay limited to the reviewed package factory and capability-specific command, tool, subagent, MCP, and static Hook providers', + 'OpenCode adapter public API budget must stay limited to the reviewed package factory and capability-specific command, configured Skill root, tool, subagent, MCP, and static Hook providers', ); } for (const entry of opencodeAdapterPublicApiRule.allowedSymbolEntries) { diff --git a/src/crates/adapters/claude-code-adapter/src/command_source.rs b/src/crates/adapters/claude-code-adapter/src/command_source.rs index 73f6ec5de..7af5d77c6 100644 --- a/src/crates/adapters/claude-code-adapter/src/command_source.rs +++ b/src/crates/adapters/claude-code-adapter/src/command_source.rs @@ -1,11 +1,14 @@ use bitfun_product_domains::external_sources::{ - EcosystemId, ExpandedPromptCommand, ExternalSourceAssetKind, ExternalSourceContext, - ExternalSourceDiagnostic, ExternalSourceHealth, ExternalSourceProviderError, - ExternalSourceRecord, ExternalSourceScope, ExternalWatchRoot, PromptCommandAvailability, - PromptCommandDefinition, PromptCommandProviderIdentity, PromptCommandProviderSnapshot, - PromptCommandSourceProvider, SourceKey, SourceQualifiedCommandId, + EcosystemId, ExternalSourceAssetKind, ExternalSourceContext, ExternalSourceDiagnostic, + ExternalSourceHealth, ExternalSourceProviderError, ExternalSourceRecord, ExternalSourceScope, + ExternalWatchRoot, PromptCommandAvailability, PromptCommandDefinition, PromptCommandExpansion, + PromptCommandProviderIdentity, PromptCommandProviderSnapshot, PromptCommandSourceProvider, + SourceKey, SourceQualifiedCommandId, }; -use bitfun_services_core::markdown::{expand_prompt_template_arguments, FrontMatterMarkdown}; +use bitfun_services_core::markdown::{ + expand_prompt_template_arguments, prompt_template_expansion_upper_bound, FrontMatterMarkdown, +}; +use bitfun_services_core::workspace_text::normalize_workspace_relative_path; use bitfun_static_hook_support::{ collect_bounded_regular_files, read_bounded_text, BoundedDirectoryWalkError, BoundedDirectoryWalkLimits, BoundedTextRead, @@ -22,6 +25,7 @@ const ECOSYSTEM_ID: &str = "claude-code"; const MAX_COMMAND_FILES: usize = 2048; const MAX_COMMAND_FILE_BYTES: usize = 256 * 1024; const MAX_TOTAL_TEMPLATE_BYTES: usize = 8 * 1024 * 1024; +const MAX_EXPANDED_COMMAND_BYTES: usize = 1024 * 1024; #[derive(Debug, Clone)] pub struct ClaudeCodeCommandProviderOptions { @@ -220,7 +224,7 @@ impl PromptCommandSourceProvider for ClaudeCodeCommandProvider { &self, command: &PromptCommandDefinition, arguments: &str, - ) -> Result { + ) -> Result { if command.id.source.provider_id.as_str() != PROVIDER_ID { return Err(ExternalSourceProviderError::new( "claude.command.identity_mismatch", @@ -229,9 +233,21 @@ impl PromptCommandSourceProvider for ClaudeCodeCommandProvider { )); } match &command.availability { - PromptCommandAvailability::Available => Ok(ExpandedPromptCommand { - content: expand_prompt_template_arguments(&command.template, arguments), - }), + PromptCommandAvailability::Available => { + if prompt_template_expansion_upper_bound(&command.template, arguments) + .is_none_or(|size| size > MAX_EXPANDED_COMMAND_BYTES) + { + return Err(ExternalSourceProviderError::new( + "claude.command.expansion_too_large", + "expanded command would exceed the 1048576 byte limit", + false, + )); + } + Ok(PromptCommandExpansion { + content: expand_prompt_template_arguments(&command.template, arguments), + workspace_file_references: literal_file_references(&command.template), + }) + } PromptCommandAvailability::Restricted { reason, .. } | PromptCommandAvailability::Invalid { reason } => { Err(ExternalSourceProviderError::new( @@ -627,12 +643,10 @@ fn command_definition( if shell_regex().is_match(&input.template) { required_capabilities.push("command.shell".to_string()); } - if file_regex().is_match(&input.template) { - required_capabilities.push("command.file_reference".to_string()); - } if dynamic_variable_regex().is_match(&input.template) { required_capabilities.push("command.dynamic_variable".to_string()); } + required_capabilities.extend(file_reference_capabilities(&input.template)); for field in input.unsupported_fields { let capability = match field.as_str() { "model" => "command.model".to_string(), @@ -699,6 +713,36 @@ fn file_regex() -> &'static Regex { }) } +fn literal_file_references(template: &str) -> Vec { + let mut seen = BTreeSet::new(); + file_regex() + .captures_iter(template) + .filter_map(|capture| capture.get(1).map(|value| value.as_str())) + .filter(|path| !is_dynamic_file_reference(path)) + .filter_map(|path| normalize_workspace_relative_path(path).ok()) + .filter(|path| seen.insert(path.clone())) + .collect() +} + +fn file_reference_capabilities(template: &str) -> Vec { + let mut capabilities = Vec::new(); + for path in file_regex() + .captures_iter(template) + .filter_map(|capture| capture.get(1).map(|value| value.as_str())) + { + if is_dynamic_file_reference(path) { + capabilities.push("command.file_reference.dynamic".to_string()); + } else if normalize_workspace_relative_path(path).is_err() { + capabilities.push("command.file_reference.unsafe_path".to_string()); + } + } + capabilities +} + +fn is_dynamic_file_reference(path: &str) -> bool { + path.contains('$') || path.contains('{') || path.contains('}') +} + fn dynamic_variable_regex() -> &'static Regex { static REGEX: OnceLock = OnceLock::new(); REGEX.get_or_init(|| { diff --git a/src/crates/adapters/claude-code-adapter/tests/command_source.rs b/src/crates/adapters/claude-code-adapter/tests/command_source.rs index d85040562..3deb21f30 100644 --- a/src/crates/adapters/claude-code-adapter/tests/command_source.rs +++ b/src/crates/adapters/claude-code-adapter/tests/command_source.rs @@ -185,13 +185,77 @@ fn dynamic_and_behavioral_commands_are_visible_but_restricted() { panic!("dynamic Claude command must be restricted") }; assert!(required_capabilities.contains(&"command.shell".to_string())); - assert!(required_capabilities.contains(&"command.file_reference".to_string())); + assert!(!required_capabilities.contains(&"command.file_reference".to_string())); assert!(required_capabilities.contains(&"command.model".to_string())); assert!(required_capabilities.contains(&"command.allowed_tools".to_string())); assert!(required_capabilities.contains(&"command.dynamic_variable".to_string())); assert!(provider.expand(command, "now").is_err()); } +#[test] +fn literal_file_references_are_prepared_without_scanning_arguments() { + let fixture = Fixture::new(); + write( + fixture.user_claude.join("commands/review.md"), + "Review @src/lib.rs, @src/lib.rs, and @docs/guide.md for $ARGUMENTS", + ); + + let provider = fixture.provider(); + let snapshot = provider.discover(&fixture.context()).unwrap(); + let command = &snapshot.commands[0]; + assert!(matches!( + command.availability, + PromptCommandAvailability::Available + )); + + let expansion = provider + .expand(command, "@arguments/are-not-files.md") + .unwrap(); + assert_eq!( + expansion.workspace_file_references, + ["src/lib.rs", "docs/guide.md"] + ); + assert_eq!( + expansion.content, + "Review @src/lib.rs, @src/lib.rs, and @docs/guide.md for @arguments/are-not-files.md" + ); +} + +#[test] +fn dynamic_and_unsafe_file_references_are_visible_but_restricted() { + let fixture = Fixture::new(); + write( + fixture.user_claude.join("commands/dynamic.md"), + "Review @src/$1.rs", + ); + write( + fixture.user_claude.join("commands/unsafe.md"), + "Review @/etc/passwd", + ); + + let provider = fixture.provider(); + let snapshot = provider.discover(&fixture.context()).unwrap(); + for (name, capability) in [ + ("dynamic", "command.file_reference.dynamic"), + ("unsafe", "command.file_reference.unsafe_path"), + ] { + let command = snapshot + .commands + .iter() + .find(|command| command.name == name) + .unwrap(); + let PromptCommandAvailability::Restricted { + required_capabilities, + .. + } = &command.availability + else { + panic!("{name} file reference must be restricted") + }; + assert!(required_capabilities.contains(&capability.to_string())); + assert!(provider.expand(command, "").is_err()); + } +} + #[test] fn safe_arguments_expand_and_description_only_changes_keep_behavior_version() { let fixture = Fixture::new(); @@ -221,6 +285,26 @@ fn safe_arguments_expand_and_description_only_changes_keep_behavior_version() { assert_eq!(updated.commands[0].description, "Updated description"); } +#[test] +fn rejects_argument_expansion_before_repeated_placeholders_can_overallocate() { + let fixture = Fixture::new(); + write( + fixture.user_claude.join("commands/large.md"), + &"$ARGUMENTS".repeat(1024), + ); + let provider = fixture.provider(); + let snapshot = provider.discover(&fixture.context()).unwrap(); + let command = snapshot + .commands + .iter() + .find(|command| command.name == "large") + .unwrap(); + + let error = provider.expand(command, &"x".repeat(2048)).unwrap_err(); + + assert_eq!(error.code, "claude.command.expansion_too_large"); +} + #[test] fn arguments_without_a_placeholder_use_claude_codes_arguments_section() { let fixture = Fixture::new(); diff --git a/src/crates/adapters/opencode-adapter/src/command_source.rs b/src/crates/adapters/opencode-adapter/src/command_source.rs index 55ba31231..f89c05ed7 100644 --- a/src/crates/adapters/opencode-adapter/src/command_source.rs +++ b/src/crates/adapters/opencode-adapter/src/command_source.rs @@ -4,14 +4,15 @@ use crate::local_source_paths::{ LocalConfigDirectoryKind, }; use bitfun_product_domains::external_sources::{ - EcosystemId, ExpandedPromptCommand, ExternalSourceAssetKind, ExternalSourceContext, - ExternalSourceDiagnostic, ExternalSourceHealth, ExternalSourceProviderError, - ExternalSourceRecord, ExternalSourceScope, ExternalWatchRoot, PromptCommandAvailability, - PromptCommandDefinition, PromptCommandProviderIdentity, PromptCommandProviderSnapshot, - PromptCommandSourceProvider, SourceKey, SourceQualifiedCommandId, + EcosystemId, ExternalSourceAssetKind, ExternalSourceContext, ExternalSourceDiagnostic, + ExternalSourceHealth, ExternalSourceProviderError, ExternalSourceRecord, ExternalSourceScope, + ExternalWatchRoot, PromptCommandAvailability, PromptCommandDefinition, PromptCommandExpansion, + PromptCommandProviderIdentity, PromptCommandProviderSnapshot, PromptCommandSourceProvider, + SourceKey, SourceQualifiedCommandId, }; pub(crate) use bitfun_services_core::jsonc::strip_jsonc; -use bitfun_services_core::markdown::FrontMatterMarkdown; +use bitfun_services_core::markdown::{prompt_template_expansion_upper_bound, FrontMatterMarkdown}; +use bitfun_services_core::workspace_text::normalize_workspace_relative_path; use bitfun_static_hook_support::{ collect_bounded_regular_files, read_bounded_text, BoundedDirectoryWalkError, BoundedDirectoryWalkLimits, BoundedTextRead, @@ -30,6 +31,7 @@ const MAX_COMMAND_FILES: usize = 2048; const MAX_COMMAND_FILE_BYTES: usize = 256 * 1024; const MAX_COMMAND_TEMPLATE_BYTES: usize = 8 * 1024 * 1024; const MAX_CONFIG_FILE_BYTES: usize = 1024 * 1024; +const MAX_EXPANDED_COMMAND_BYTES: usize = 1024 * 1024; #[derive(Debug, Clone)] pub struct OpenCodeCommandProviderOptions { @@ -73,7 +75,7 @@ impl OpenCodeCommandProvider { Self { options } } - fn discover_layers(&self, context: &ExternalSourceContext) -> Vec { + fn discover_layers(&self, workspace_root: Option<&Path>) -> Vec { let mut layers = Vec::new(); // Phase 1: global JSON configuration. push_config_file_layer( @@ -99,7 +101,7 @@ impl OpenCodeCommandProvider { } // Phase 3: project JSON configuration, root first. if self.options.project_config_enabled { - if let Some(workspace_root) = &context.workspace_root { + if let Some(workspace_root) = workspace_root { let project_root = find_project_root(workspace_root); for directory in project_config_directories(&project_root, workspace_root) { push_config_directory_layers( @@ -114,9 +116,7 @@ impl OpenCodeCommandProvider { // Phase 4: ConfigPaths.directories. OpenCode keeps the first physical // directory when an environment path aliases an earlier entry. let project_directories = if self.options.project_config_enabled { - context - .workspace_root - .as_ref() + workspace_root .map(|workspace_root| { project_asset_directories(&find_project_root(workspace_root), workspace_root) }) @@ -161,6 +161,22 @@ impl OpenCodeCommandProvider { } deduplicate_layers_keep_last(layers) } + + pub(crate) fn config_file_layers( + &self, + workspace_root: Option<&Path>, + ) -> Vec { + self.discover_layers(workspace_root) + .into_iter() + .filter_map(|layer| match layer.kind { + SourceLayerKind::ConfigFile(path) => Some(OpenCodeConfigFileLayer { + path, + scope: layer.scope, + }), + SourceLayerKind::CommandDirectory(_) => None, + }) + .collect() + } } impl Default for OpenCodeCommandProvider { @@ -197,7 +213,7 @@ impl PromptCommandSourceProvider for OpenCodeCommandProvider { let mut unavailable_command_ids = Vec::new(); let mut provider_template_bytes = 0usize; - for layer in self.discover_layers(context) { + for layer in self.discover_layers(context.workspace_root.as_deref()) { let parsed = match &layer.kind { SourceLayerKind::ConfigFile(path) => parse_config_file(path), SourceLayerKind::CommandDirectory(path) => parse_command_directory(path), @@ -309,7 +325,7 @@ impl PromptCommandSourceProvider for OpenCodeCommandProvider { &self, command: &PromptCommandDefinition, arguments: &str, - ) -> Result { + ) -> Result { if command.id.source.provider_id.as_str() != PROVIDER_ID { return Err(ExternalSourceProviderError::new( "opencode.command.identity_mismatch", @@ -318,9 +334,21 @@ impl PromptCommandSourceProvider for OpenCodeCommandProvider { )); } match &command.availability { - PromptCommandAvailability::Available => Ok(ExpandedPromptCommand { - content: expand_template(&command.template, arguments), - }), + PromptCommandAvailability::Available => { + if prompt_template_expansion_upper_bound(&command.template, arguments) + .is_none_or(|size| size > MAX_EXPANDED_COMMAND_BYTES) + { + return Err(ExternalSourceProviderError::new( + "opencode.command.expansion_too_large", + "expanded command would exceed the 1048576 byte limit", + false, + )); + } + Ok(PromptCommandExpansion { + content: expand_template(&command.template, arguments), + workspace_file_references: literal_file_references(&command.template), + }) + } PromptCommandAvailability::Restricted { reason, .. } | PromptCommandAvailability::Invalid { reason } => { Err(ExternalSourceProviderError::new( @@ -405,6 +433,12 @@ struct SourceLayer { source_kind: &'static str, } +#[derive(Debug, Clone)] +pub(crate) struct OpenCodeConfigFileLayer { + pub(crate) path: PathBuf, + pub(crate) scope: ExternalSourceScope, +} + #[derive(Debug)] enum SourceLayerKind { ConfigFile(PathBuf), @@ -872,9 +906,6 @@ fn command_definition( if shell_regex().is_match(&input.template) { required_capabilities.push("command.shell".to_string()); } - if file_regex().is_match(&input.template) { - required_capabilities.push("command.file_reference".to_string()); - } if input.agent.is_some() { required_capabilities.push("command.agent".to_string()); } @@ -890,6 +921,9 @@ fn command_definition( if config_variable_regex().is_match(&input.template) { required_capabilities.push("command.config_variable".to_string()); } + required_capabilities.extend(file_reference_capabilities(&input.template)); + required_capabilities.sort(); + required_capabilities.dedup(); let availability = if required_capabilities.is_empty() { PromptCommandAvailability::Available } else { @@ -1032,6 +1066,36 @@ fn file_regex() -> &'static Regex { }) } +fn literal_file_references(template: &str) -> Vec { + let mut seen = BTreeSet::new(); + file_regex() + .captures_iter(template) + .filter_map(|capture| capture.get(1).map(|value| value.as_str())) + .filter(|path| !is_dynamic_file_reference(path)) + .filter_map(|path| normalize_workspace_relative_path(path).ok()) + .filter(|path| seen.insert(path.clone())) + .collect() +} + +fn file_reference_capabilities(template: &str) -> Vec { + let mut capabilities = Vec::new(); + for path in file_regex() + .captures_iter(template) + .filter_map(|capture| capture.get(1).map(|value| value.as_str())) + { + if is_dynamic_file_reference(path) { + capabilities.push("command.file_reference.dynamic".to_string()); + } else if normalize_workspace_relative_path(path).is_err() { + capabilities.push("command.file_reference.unsafe_path".to_string()); + } + } + capabilities +} + +fn is_dynamic_file_reference(path: &str) -> bool { + path.contains('$') || path.contains('{') || path.contains('}') +} + fn config_variable_regex() -> &'static Regex { static REGEX: OnceLock = OnceLock::new(); REGEX diff --git a/src/crates/adapters/opencode-adapter/src/lib.rs b/src/crates/adapters/opencode-adapter/src/lib.rs index a8fa37fe6..d69d46cbb 100644 --- a/src/crates/adapters/opencode-adapter/src/lib.rs +++ b/src/crates/adapters/opencode-adapter/src/lib.rs @@ -12,6 +12,7 @@ mod hook_contributions; mod hook_source; mod local_source_paths; mod mcp_source; +mod skill_source; mod source_adapter; mod tool_source; @@ -19,5 +20,8 @@ pub use agent_source::{OpenCodeSubagentProvider, OpenCodeSubagentProviderOptions pub use command_source::{OpenCodeCommandProvider, OpenCodeCommandProviderOptions}; pub use hook_source::{OpenCodeHookProvider, OpenCodeHookProviderOptions}; pub use mcp_source::{OpenCodeMcpProvider, OpenCodeMcpProviderOptions}; +pub use skill_source::{ + OpenCodeConfiguredSkillRoot, OpenCodeSkillRootProvider, OpenCodeSkillRootProviderOptions, +}; pub use source_adapter::load_opencode_package_adapter; pub use tool_source::{OpenCodeToolProvider, OpenCodeToolProviderOptions}; diff --git a/src/crates/adapters/opencode-adapter/src/skill_source.rs b/src/crates/adapters/opencode-adapter/src/skill_source.rs new file mode 100644 index 000000000..2007b3c86 --- /dev/null +++ b/src/crates/adapters/opencode-adapter/src/skill_source.rs @@ -0,0 +1,189 @@ +use crate::command_source::{strip_jsonc, OpenCodeCommandProvider, OpenCodeCommandProviderOptions}; +use crate::local_source_paths::find_project_root; +use bitfun_product_domains::external_sources::ExternalSourceScope; +use bitfun_services_core::bounded_fs::{read_bounded_text, BoundedTextRead}; +use serde_json::Value; +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +const MAX_CONFIG_FILE_BYTES: usize = 1024 * 1024; +const MAX_CONFIGURED_SKILL_ROOTS: usize = 64; + +#[derive(Debug, Clone)] +pub struct OpenCodeSkillRootProviderOptions { + pub command: OpenCodeCommandProviderOptions, + pub home_dir: Option, +} + +impl OpenCodeSkillRootProviderOptions { + pub fn from_environment() -> Self { + Self { + command: OpenCodeCommandProviderOptions::from_environment(), + home_dir: dirs::home_dir(), + } + } +} + +impl Default for OpenCodeSkillRootProviderOptions { + fn default() -> Self { + Self::from_environment() + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OpenCodeConfiguredSkillRoot { + pub path: PathBuf, + pub scope: ExternalSourceScope, + pub precedence: usize, +} + +pub struct OpenCodeSkillRootProvider { + command_provider: OpenCodeCommandProvider, + home_dir: Option, +} + +impl OpenCodeSkillRootProvider { + pub fn new(options: OpenCodeSkillRootProviderOptions) -> Self { + Self { + command_provider: OpenCodeCommandProvider::new(options.command), + home_dir: options.home_dir, + } + } + + pub fn discover(&self, workspace_root: Option<&Path>) -> Vec { + let canonical_workspace = workspace_root + .map(find_project_root) + .and_then(|path| dunce::canonicalize(path).ok()); + let canonical_home = self + .home_dir + .as_deref() + .and_then(|path| dunce::canonicalize(path).ok()); + let mut configured_paths = Vec::new(); + let mut precedence = 0usize; + + for layer in self.command_provider.config_file_layers(workspace_root) { + let Some(paths) = read_local_skill_paths(&layer.path) else { + continue; + }; + for value in paths { + let current_precedence = precedence; + precedence = precedence.saturating_add(1); + configured_paths.push((value, layer.scope, current_precedence)); + } + } + + let mut contributions = Vec::new(); + for (value, source_scope, current_precedence) in configured_paths + .into_iter() + .rev() + .take(MAX_CONFIGURED_SKILL_ROOTS) + .collect::>() + .into_iter() + .rev() + { + let Some(path) = + resolve_configured_path(&value, workspace_root, self.home_dir.as_deref()) + else { + continue; + }; + let Ok(path) = dunce::canonicalize(path) else { + continue; + }; + if !path.is_dir() { + continue; + } + let workspace_scoped = canonical_workspace + .as_ref() + .is_some_and(|workspace| path.starts_with(workspace)); + let home_scoped = canonical_home + .as_ref() + .is_some_and(|home| path.starts_with(home)); + let scope = match source_scope { + ExternalSourceScope::Project | ExternalSourceScope::WorkspaceLocal + if workspace_scoped => + { + ExternalSourceScope::Project + } + ExternalSourceScope::UserGlobal if workspace_scoped => ExternalSourceScope::Project, + ExternalSourceScope::UserGlobal if home_scoped => ExternalSourceScope::UserGlobal, + _ => continue, + }; + contributions.push(OpenCodeConfiguredSkillRoot { + path, + scope, + precedence: current_precedence, + }); + } + + let mut seen = BTreeSet::new(); + contributions + .into_iter() + .filter(|root| seen.insert(root.path.clone())) + .collect() + } +} + +impl Default for OpenCodeSkillRootProvider { + fn default() -> Self { + Self::new(OpenCodeSkillRootProviderOptions::default()) + } +} + +fn read_local_skill_paths(path: &Path) -> Option> { + let content = match read_bounded_text(path, MAX_CONFIG_FILE_BYTES).ok()? { + BoundedTextRead::Content(content) => content, + BoundedTextRead::TooLarge | BoundedTextRead::InvalidUtf8 => return None, + }; + let document = serde_json::from_str::(&strip_jsonc(&content)).ok()?; + match document.get("skills")? { + Value::Object(skills) => strict_string_array(skills.get("paths")) + .map(|paths| local_only_paths(paths.into_iter())), + Value::Array(skills) => skills + .iter() + .map(Value::as_str) + .collect::>>() + .map(|paths| local_only_paths(paths.into_iter())), + _ => None, + } +} + +fn strict_string_array(value: Option<&Value>) -> Option> { + let Some(value) = value else { + return Some(Vec::new()); + }; + value.as_array()?.iter().map(Value::as_str).collect() +} + +fn local_only_paths<'a>(paths: impl Iterator) -> Vec { + paths + .filter(|value| !is_remote_url(value)) + .map(str::to_string) + .collect() +} + +fn is_remote_url(value: &str) -> bool { + url::Url::parse(value).is_ok_and(|url| matches!(url.scheme(), "http" | "https")) +} + +fn resolve_configured_path( + value: &str, + workspace_root: Option<&Path>, + home_dir: Option<&Path>, +) -> Option { + let value = value.trim(); + if value.is_empty() || is_remote_url(value) || value.contains('\0') { + return None; + } + if let Some(relative) = value + .strip_prefix("~/") + .or_else(|| value.strip_prefix("~\\")) + { + return home_dir.map(|home| home.join(relative)); + } + let path = PathBuf::from(value); + if path.is_absolute() { + Some(path) + } else { + workspace_root.map(|workspace| workspace.join(path)) + } +} diff --git a/src/crates/adapters/opencode-adapter/tests/opencode_command_adapter.rs b/src/crates/adapters/opencode-adapter/tests/opencode_command_adapter.rs index 29fc986da..3be7012b4 100644 --- a/src/crates/adapters/opencode-adapter/tests/opencode_command_adapter.rs +++ b/src/crates/adapters/opencode-adapter/tests/opencode_command_adapter.rs @@ -351,8 +351,10 @@ fn unsupported_expansion_features_restrict_the_whole_command() { fixture.user_config.join("opencode.json"), r#"{ "command": { - "shell": {"template":"Run !`git status`"}, - "file": {"template":"Review @src/main.rs"}, + "shell": {"template":"Run !`git status` and review @src/main.rs"}, + "file": {"template":"Review @src/main.rs, @src/main.rs, and @docs/guide.md with $ARGUMENTS"}, + "dynamic-file": {"template":"Review @src/$1.rs"}, + "unsafe-file": {"template":"Review @/etc/passwd"}, "config-var": {"template":"Review {env:HOME}"}, "agent": {"template":"Delegate this", "agent":"explore"}, "subtask": {"template":"Delegate this", "subtask":true} @@ -366,7 +368,14 @@ fn unsupported_expansion_features_restrict_the_whole_command() { .sources .iter() .any(|source| source.health == ExternalSourceHealth::Partial)); - for name in ["shell", "file", "config-var", "agent", "subtask"] { + for name in [ + "shell", + "dynamic-file", + "unsafe-file", + "config-var", + "agent", + "subtask", + ] { let command = snapshot .commands .iter() @@ -378,6 +387,53 @@ fn unsupported_expansion_features_restrict_the_whole_command() { )); assert!(provider.expand(command, "").is_err()); } + let dynamic_file = snapshot + .commands + .iter() + .find(|command| command.name == "dynamic-file") + .unwrap(); + let PromptCommandAvailability::Restricted { + required_capabilities, + .. + } = &dynamic_file.availability + else { + panic!("dynamic file references must be restricted") + }; + assert!(required_capabilities.contains(&"command.file_reference.dynamic".to_string())); + let unsafe_file = snapshot + .commands + .iter() + .find(|command| command.name == "unsafe-file") + .unwrap(); + let PromptCommandAvailability::Restricted { + required_capabilities, + .. + } = &unsafe_file.availability + else { + panic!("unsafe file references must be restricted") + }; + assert!(required_capabilities.contains(&"command.file_reference.unsafe_path".to_string())); + + let file_command = snapshot + .commands + .iter() + .find(|command| command.name == "file") + .unwrap(); + assert!(matches!( + file_command.availability, + PromptCommandAvailability::Available + )); + let expansion = provider + .expand(file_command, "@arguments/are-not-files.md") + .unwrap(); + assert_eq!( + expansion.workspace_file_references, + ["src/main.rs", "docs/guide.md"] + ); + assert_eq!( + expansion.content, + "Review @src/main.rs, @src/main.rs, and @docs/guide.md with @arguments/are-not-files.md" + ); } #[test] @@ -508,6 +564,26 @@ fn expands_arguments_and_positions_using_the_frozen_opencode_semantics() { ); } +#[test] +fn rejects_argument_expansion_before_repeated_placeholders_can_overallocate() { + let fixture = Fixture::new(); + write( + fixture.user_config.join("commands/large.md"), + &"$ARGUMENTS".repeat(1024), + ); + let provider = fixture.provider(); + let snapshot = provider.discover(&fixture.context()).unwrap(); + let command = snapshot + .commands + .iter() + .find(|command| command.name == "large") + .unwrap(); + + let error = provider.expand(command, &"x".repeat(2048)).unwrap_err(); + + assert_eq!(error.code, "opencode.command.expansion_too_large"); +} + #[test] fn deleting_the_winning_file_reveals_the_next_opencode_source() { let fixture = Fixture::new(); @@ -584,6 +660,22 @@ fn semantically_invalid_known_command_is_marked_unavailable() { .any(|command_id| command_id.local_id.as_str() == "review")); } +#[test] +fn malformed_skills_field_does_not_hide_other_valid_command_contributions() { + let fixture = Fixture::new(); + write( + fixture.user_config.join("opencode.json"), + r#"{"skills":["valid",42],"command":{"review":{"template":"review this"}}}"#, + ); + + let snapshot = fixture.provider().discover(&fixture.context()).unwrap(); + + assert!(snapshot + .commands + .iter() + .any(|command| command.id.local_id.as_str() == "review")); +} + #[test] fn invalid_command_directory_shape_is_unavailable_not_a_stable_empty_source() { let fixture = Fixture::new(); diff --git a/src/crates/adapters/opencode-adapter/tests/opencode_skill_roots.rs b/src/crates/adapters/opencode-adapter/tests/opencode_skill_roots.rs new file mode 100644 index 000000000..a560f7188 --- /dev/null +++ b/src/crates/adapters/opencode-adapter/tests/opencode_skill_roots.rs @@ -0,0 +1,202 @@ +use bitfun_opencode_adapter::{ + OpenCodeCommandProviderOptions, OpenCodeSkillRootProvider, OpenCodeSkillRootProviderOptions, +}; +use bitfun_product_domains::external_sources::ExternalSourceScope; +use std::fs; +use std::path::{Path, PathBuf}; + +struct Fixture { + _temp: tempfile::TempDir, + home: PathBuf, + user_config: PathBuf, + project: PathBuf, + opened_directory: PathBuf, +} + +impl Fixture { + fn new() -> Self { + let temp = tempfile::tempdir().unwrap(); + let home = temp.path().join("home"); + let user_config = home.join(".config/opencode"); + let project = temp.path().join("project"); + let opened_directory = project.join("packages/app"); + fs::create_dir_all(&user_config).unwrap(); + fs::create_dir_all(project.join(".git")).unwrap(); + fs::create_dir_all(&opened_directory).unwrap(); + Self { + _temp: temp, + home, + user_config, + project, + opened_directory, + } + } + + fn provider(&self) -> OpenCodeSkillRootProvider { + OpenCodeSkillRootProvider::new(OpenCodeSkillRootProviderOptions { + command: OpenCodeCommandProviderOptions { + user_config_dir: self.user_config.clone(), + legacy_user_config_dir: Some(self.home.join(".opencode")), + explicit_config_file: None, + explicit_config_dir: None, + project_config_enabled: true, + }, + home_dir: Some(self.home.clone()), + }) + } +} + +fn write(path: impl AsRef, contents: &str) { + let path = path.as_ref(); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); +} + +#[test] +fn accumulates_v1_and_current_local_skill_paths_in_config_source_order() { + let fixture = Fixture::new(); + let user_skill = fixture.home.join("shared-skills"); + let project_skill = fixture.project.join("project-skills"); + let nested_skill = fixture.opened_directory.join("nested-skills"); + for path in [&user_skill, &project_skill, &nested_skill] { + fs::create_dir_all(path).unwrap(); + } + write( + fixture.user_config.join("opencode.json"), + r#"{"skills":{"paths":["~/shared-skills"],"urls":["https://example.test/skills"]}}"#, + ); + write( + fixture.project.join("opencode.json"), + r#"{"skills":{"paths":["../../project-skills"]}}"#, + ); + write( + fixture.opened_directory.join("opencode.jsonc"), + r#"{"skills":["nested-skills", "https://example.test/ignored"]}"#, + ); + + let roots = fixture.provider().discover(Some(&fixture.opened_directory)); + + assert_eq!(roots.len(), 3); + assert_eq!(roots[0].path, dunce::canonicalize(user_skill).unwrap()); + assert_eq!(roots[0].scope, ExternalSourceScope::UserGlobal); + assert_eq!(roots[1].path, dunce::canonicalize(project_skill).unwrap()); + assert_eq!(roots[1].scope, ExternalSourceScope::Project); + assert_eq!(roots[2].path, dunce::canonicalize(nested_skill).unwrap()); + assert_eq!(roots[2].scope, ExternalSourceScope::Project); + assert!(roots + .windows(2) + .all(|pair| pair[0].precedence < pair[1].precedence)); +} + +#[test] +fn rejects_roots_outside_the_config_source_boundary() { + let fixture = Fixture::new(); + let arbitrary = fixture._temp.path().join("arbitrary"); + let home_skill = fixture.home.join("allowed-home"); + let workspace_skill = fixture.project.join("allowed-project"); + for path in [&arbitrary, &home_skill, &workspace_skill] { + fs::create_dir_all(path).unwrap(); + } + write( + fixture.user_config.join("opencode.json"), + &format!( + r#"{{"skills":{{"paths":["{}", "~/allowed-home"]}}}}"#, + arbitrary.to_string_lossy().replace('\\', "\\\\") + ), + ); + write( + fixture.project.join("opencode.json"), + &format!( + r#"{{"skills":{{"paths":["{}", "../../allowed-project"]}}}}"#, + home_skill.to_string_lossy().replace('\\', "\\\\") + ), + ); + + let roots = fixture.provider().discover(Some(&fixture.opened_directory)); + let canonical_home_skill = dunce::canonicalize(&home_skill).unwrap(); + let canonical_workspace_skill = dunce::canonicalize(&workspace_skill).unwrap(); + let canonical_arbitrary = dunce::canonicalize(&arbitrary).unwrap(); + + assert_eq!(roots.len(), 2); + assert!(roots.iter().any(|root| root.path == canonical_home_skill)); + assert!(roots + .iter() + .any(|root| root.path == canonical_workspace_skill)); + assert!(roots.iter().all(|root| root.path != canonical_arbitrary)); +} + +#[test] +fn deduplicates_canonical_roots_while_retaining_the_first_source_position() { + let fixture = Fixture::new(); + let root = fixture.project.join("shared"); + fs::create_dir_all(&root).unwrap(); + write( + fixture.project.join("opencode.json"), + r#"{"skills":{"paths":["../../shared"]}}"#, + ); + write( + fixture.opened_directory.join("opencode.json"), + r#"{"skills":{"paths":["../../shared"]}}"#, + ); + + let roots = fixture.provider().discover(Some(&fixture.opened_directory)); + + assert_eq!(roots.len(), 1); + assert_eq!(roots[0].path, dunce::canonicalize(root).unwrap()); + assert_eq!(roots[0].precedence, 0); +} + +#[test] +fn caps_configured_roots_by_retaining_the_latest_entries() { + let fixture = Fixture::new(); + let paths = (0..65) + .map(|index| { + fs::create_dir_all(fixture.project.join(format!("skills-{index}"))).unwrap(); + format!("../../skills-{index}") + }) + .collect::>(); + write( + fixture.project.join("opencode.json"), + &serde_json::json!({"skills": {"paths": paths}}).to_string(), + ); + + let roots = fixture.provider().discover(Some(&fixture.opened_directory)); + + assert_eq!(roots.len(), 64); + assert!(roots.iter().all(|root| !root.path.ends_with("skills-0"))); + assert!(roots.iter().any(|root| root.path.ends_with("skills-64"))); +} + +#[test] +fn rejects_a_malformed_skills_list_instead_of_partially_loading_it() { + let fixture = Fixture::new(); + fs::create_dir_all(fixture.project.join("valid-skills")).unwrap(); + write( + fixture.project.join("opencode.json"), + r#"{"skills":["valid-skills", 42]}"#, + ); + + let roots = fixture.provider().discover(Some(&fixture.opened_directory)); + + assert!(roots.is_empty()); +} + +#[test] +fn no_workspace_never_interprets_a_relative_configured_root_locally() { + let fixture = Fixture::new(); + fs::create_dir_all(fixture.home.join("global-skills")).unwrap(); + fs::create_dir_all(fixture.project.join("relative-skills")).unwrap(); + write( + fixture.user_config.join("opencode.json"), + r#"{"skills":["relative-skills", "~/global-skills"]}"#, + ); + + let roots = fixture.provider().discover(None); + + assert_eq!(roots.len(), 1); + assert_eq!( + roots[0].path, + dunce::canonicalize(fixture.home.join("global-skills")).unwrap() + ); + assert_eq!(roots[0].scope, ExternalSourceScope::UserGlobal); +} diff --git a/src/crates/adapters/static-hook-support/Cargo.toml b/src/crates/adapters/static-hook-support/Cargo.toml index d58b4d77d..3159aae70 100644 --- a/src/crates/adapters/static-hook-support/Cargo.toml +++ b/src/crates/adapters/static-hook-support/Cargo.toml @@ -11,6 +11,7 @@ crate-type = ["rlib"] [dependencies] bitfun-product-domains = { path = "../../contracts/product-domains", default-features = false, features = ["external-sources"] } +bitfun-services-core = { path = "../../services/services-core", default-features = false } serde_json = { workspace = true } toml = { workspace = true } hex = { workspace = true } diff --git a/src/crates/adapters/static-hook-support/src/lib.rs b/src/crates/adapters/static-hook-support/src/lib.rs index 2575f8a5b..0b465d899 100644 --- a/src/crates/adapters/static-hook-support/src/lib.rs +++ b/src/crates/adapters/static-hook-support/src/lib.rs @@ -8,167 +8,18 @@ use bitfun_product_domains::external_hook_import::{ MAX_EXTERNAL_HOOK_IMPORT_ASSET_BYTES, MAX_EXTERNAL_HOOK_IMPORT_ASSET_DEPTH, MAX_EXTERNAL_HOOK_IMPORT_TOTAL_ASSET_BYTES, }; +pub use bitfun_services_core::bounded_fs::{ + collect_bounded_regular_files, read_bounded_file, read_bounded_text, BoundedDirectoryWalkError, + BoundedDirectoryWalkLimit, BoundedDirectoryWalkLimits, BoundedFileRead, BoundedTextRead, +}; use serde_json::Value; use sha2::{Digest, Sha256}; use std::collections::{BTreeMap, BTreeSet}; -use std::io::Read; use std::path::{Component, Path, PathBuf}; const MAX_MATCHER_BYTES: usize = 512; const MAX_EVENT_NAME_BYTES: usize = 160; -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum BoundedFileRead { - Content(Vec), - TooLarge, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum BoundedTextRead { - Content(String), - TooLarge, - InvalidUtf8, -} - -/// Reads at most `max_bytes + 1` bytes so a file changed between metadata and -/// read cannot cause an unbounded allocation. -pub fn read_bounded_file(path: &Path, max_bytes: usize) -> std::io::Result { - let file = std::fs::File::open(path)?; - let read_limit = max_bytes.saturating_add(1) as u64; - let mut bytes = Vec::with_capacity(max_bytes.min(64 * 1024).saturating_add(1)); - file.take(read_limit).read_to_end(&mut bytes)?; - if bytes.len() > max_bytes { - Ok(BoundedFileRead::TooLarge) - } else { - Ok(BoundedFileRead::Content(bytes)) - } -} - -/// Reads a UTF-8 text file without allocating more than `max_bytes + 1`. -pub fn read_bounded_text(path: &Path, max_bytes: usize) -> std::io::Result { - match read_bounded_file(path, max_bytes)? { - BoundedFileRead::Content(bytes) => Ok(match String::from_utf8(bytes) { - Ok(content) => BoundedTextRead::Content(content), - Err(_) => BoundedTextRead::InvalidUtf8, - }), - BoundedFileRead::TooLarge => Ok(BoundedTextRead::TooLarge), - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct BoundedDirectoryWalkLimits { - pub max_depth: usize, - pub max_entries: usize, - pub max_directories: usize, - pub max_files: usize, -} - -impl BoundedDirectoryWalkLimits { - pub fn for_file_limit(max_files: usize) -> Self { - Self { - max_depth: 32, - max_entries: max_files.saturating_mul(4).max(1), - max_directories: max_files.max(1), - max_files, - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BoundedDirectoryWalkLimit { - Depth, - Entries, - Directories, - Files, -} - -#[derive(Debug)] -pub enum BoundedDirectoryWalkError { - Io(std::io::Error), - LimitExceeded(BoundedDirectoryWalkLimit), -} - -impl std::fmt::Display for BoundedDirectoryWalkError { - fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Io(error) => write!(formatter, "{error}"), - Self::LimitExceeded(limit) => write!(formatter, "{limit:?} limit exceeded"), - } - } -} - -impl std::error::Error for BoundedDirectoryWalkError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::Io(error) => Some(error), - Self::LimitExceeded(_) => None, - } - } -} - -/// Iteratively collects matching regular files without following symlinks. -/// Limits apply to the actual traversal cost, not only to matching files. -pub fn collect_bounded_regular_files( - root: &Path, - limits: BoundedDirectoryWalkLimits, - mut matches: impl FnMut(&Path) -> bool, -) -> Result, BoundedDirectoryWalkError> { - let metadata = match std::fs::symlink_metadata(root) { - Ok(metadata) => metadata, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), - Err(error) => return Err(BoundedDirectoryWalkError::Io(error)), - }; - if !metadata.is_dir() || metadata.file_type().is_symlink() { - return Ok(Vec::new()); - } - - let mut files = Vec::new(); - let mut stack = vec![(root.to_path_buf(), 0usize)]; - let mut visited_entries = 0usize; - let mut visited_directories = 1usize; - while let Some((directory, depth)) = stack.pop() { - let entries = std::fs::read_dir(&directory).map_err(BoundedDirectoryWalkError::Io)?; - for entry in entries { - let entry = entry.map_err(BoundedDirectoryWalkError::Io)?; - visited_entries = visited_entries.saturating_add(1); - if visited_entries > limits.max_entries { - return Err(BoundedDirectoryWalkError::LimitExceeded( - BoundedDirectoryWalkLimit::Entries, - )); - } - let file_type = entry.file_type().map_err(BoundedDirectoryWalkError::Io)?; - if file_type.is_symlink() { - continue; - } - let path = entry.path(); - if file_type.is_dir() { - let next_depth = depth.saturating_add(1); - if next_depth > limits.max_depth { - return Err(BoundedDirectoryWalkError::LimitExceeded( - BoundedDirectoryWalkLimit::Depth, - )); - } - visited_directories = visited_directories.saturating_add(1); - if visited_directories > limits.max_directories { - return Err(BoundedDirectoryWalkError::LimitExceeded( - BoundedDirectoryWalkLimit::Directories, - )); - } - stack.push((path, next_depth)); - } else if file_type.is_file() && matches(&path) { - if files.len() >= limits.max_files { - return Err(BoundedDirectoryWalkError::LimitExceeded( - BoundedDirectoryWalkLimit::Files, - )); - } - files.push(path); - } - } - } - files.sort(); - Ok(files) -} - /// Distinguishes an absent path from metadata failures. Static adapters may /// ignore `NotFound`, but permission and transient filesystem failures must be /// surfaced so the coordinator can retain the last valid snapshot as stale. diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs index cacd1e1ff..ad931c6f0 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs @@ -9,6 +9,10 @@ use super::mode_overrides::{ }; use super::types::{ModeSkillInfo, SkillData, SkillInfo, SkillLocation}; use crate::agentic::workspace::WorkspaceFileSystem; +#[cfg(feature = "product-full")] +use crate::external_sources::{ + opencode_configured_skill_roots, LocalConfiguredSkillRootContribution, +}; use crate::infrastructure::get_path_manager_arc; use crate::util::errors::{BitFunError, BitFunResult}; use bitfun_agent_runtime::skills::{ @@ -22,13 +26,33 @@ use bitfun_agent_runtime::skills::{ BITFUN_USER_SKILL_SLOT, PROJECT_SKILL_KEY_PREFIX, PROJECT_SKILL_ROOTS, USER_CONFIG_SKILL_ROOTS, USER_HOME_SKILL_ROOTS, USER_SKILL_KEY_PREFIX, }; +#[cfg(feature = "product-full")] +use bitfun_services_core::bounded_fs::{collect_bounded_regular_files, BoundedDirectoryWalkLimits}; +#[cfg(feature = "product-full")] +use bitfun_services_core::bounded_fs::{is_symlink_or_reparse, read_bounded_text, BoundedTextRead}; +#[cfg(feature = "product-full")] +use bitfun_services_core::workspace_text::read_workspace_relative_text_bounded; use log::{debug, error, warn}; +#[cfg(feature = "product-full")] +use sha2::{Digest, Sha256}; use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::sync::OnceLock; use tokio::fs; use tokio::sync::RwLock; +#[cfg(feature = "product-full")] +const MAX_OPENCODE_CONFIGURED_SKILL_ROOTS: usize = 64; +#[cfg(feature = "product-full")] +const MAX_OPENCODE_CONFIGURED_SKILLS_PER_ROOT: usize = 512; +#[cfg(feature = "product-full")] +const MAX_OPENCODE_CONFIGURED_SKILL_BYTES: usize = 256 * 1024; +#[cfg(feature = "product-full")] +const MAX_OPENCODE_CONFIGURED_POLICY_BYTES: usize = 64 * 1024; +#[cfg(feature = "product-full")] +const OPENCODE_CONFIGURED_PRIORITY_BAND: usize = + MAX_OPENCODE_CONFIGURED_SKILL_ROOTS * MAX_OPENCODE_CONFIGURED_SKILLS_PER_ROOT; + /// Global Skill registry instance static SKILL_REGISTRY: OnceLock = OnceLock::new(); @@ -62,6 +86,51 @@ fn sort_remote_dir_entries(entries: &mut [crate::agentic::workspace::WorkspaceDi }); } +#[cfg(feature = "product-full")] +fn configured_opencode_source_slot(skill_dir: &Path) -> String { + let mut hasher = Sha256::new(); + hasher.update(skill_dir.to_string_lossy().as_bytes()); + let digest = hex::encode(hasher.finalize()); + format!("config.opencode.{}", &digest[..16]) +} + +#[cfg(feature = "product-full")] +fn canonical_candidate_path(candidate: &SkillCandidate) -> PathBuf { + dunce::canonicalize(&candidate.info.path) + .unwrap_or_else(|_| PathBuf::from(&candidate.info.path)) +} + +#[cfg(feature = "product-full")] +fn is_configured_opencode_source_slot(source_slot: &str) -> bool { + source_slot.starts_with("config.opencode.") +} + +#[cfg(feature = "product-full")] +fn validate_configured_opencode_skill_root( + skill_dir: &Path, + expected_source_slot: &str, +) -> Result { + if !skill_dir.is_absolute() { + return Err("configured OpenCode skill root must be absolute".to_string()); + } + let metadata = std::fs::symlink_metadata(skill_dir) + .map_err(|error| format!("failed to inspect configured OpenCode skill root: {error}"))?; + if is_symlink_or_reparse(&metadata) { + return Err( + "configured OpenCode skill root must not be a symlink or reparse point".to_string(), + ); + } + if !metadata.is_dir() { + return Err("configured OpenCode skill root must be a directory".to_string()); + } + let canonical = dunce::canonicalize(skill_dir) + .map_err(|error| format!("failed to resolve configured OpenCode skill root: {error}"))?; + if configured_opencode_source_slot(&canonical) != expected_source_slot { + return Err("configured OpenCode skill root identity changed after discovery".to_string()); + } + Ok(canonical) +} + /// Skill registry pub struct SkillRegistry { /// Cached raw user-level skills (no workspace-specific project skills). @@ -133,6 +202,77 @@ impl SkillRegistry { } } + #[cfg(feature = "product-full")] + async fn apply_configured_opencode_policy( + skill_data: &mut SkillData, + skill_dir: &Path, + source_slot: &str, + ) { + let skill_dir = match validate_configured_opencode_skill_root(skill_dir, source_slot) { + Ok(skill_dir) => skill_dir, + Err(error) => { + warn!( + "Ignoring configured OpenCode skill policy under {}: {}", + skill_dir.display(), + error + ); + return; + } + }; + let content = match read_workspace_relative_text_bounded( + &skill_dir, + "agents/openai.yaml", + MAX_OPENCODE_CONFIGURED_POLICY_BYTES, + ) + .await + { + Ok(file) => file.content, + Err(bitfun_services_core::workspace_text::WorkspaceTextReadError::NotFound) => return, + Err(error) => { + warn!( + "Ignoring configured OpenCode skill policy under {}: {}", + skill_dir.display(), + error + ); + return; + } + }; + + if let Err(error) = skill_data.apply_openai_yaml_policy(&content) { + warn!( + "Ignoring invalid configured OpenCode skill policy under {}: {}", + skill_dir.display(), + error + ); + } + } + + async fn read_local_skill_markdown(info: &SkillInfo) -> BitFunResult { + #[cfg(feature = "product-full")] + if is_configured_opencode_source_slot(&info.source_slot) { + let skill_dir = + validate_configured_opencode_skill_root(Path::new(&info.path), &info.source_slot) + .map_err(BitFunError::tool)?; + return read_workspace_relative_text_bounded( + &skill_dir, + "SKILL.md", + MAX_OPENCODE_CONFIGURED_SKILL_BYTES, + ) + .await + .map(|file| file.content) + .map_err(|error| { + BitFunError::tool(format!( + "Failed to read configured OpenCode skill file: {error}" + )) + }); + } + + let skill_md_path = PathBuf::from(&info.path).join("SKILL.md"); + fs::read_to_string(&skill_md_path) + .await + .map_err(|error| BitFunError::tool(format!("Failed to read skill file: {}", error))) + } + async fn apply_remote_openai_policy( skill_data: &mut SkillData, fs: &dyn WorkspaceFileSystem, @@ -364,14 +504,277 @@ impl SkillRegistry { debug!("Failed to install built-in skills: {}", error); } + let entries = Self::get_possible_paths_for_workspace(workspace_root); let mut skills = Vec::new(); - for entry in Self::get_possible_paths_for_workspace(workspace_root) { + for entry in entries { let mut part = Self::scan_skills_in_dir(&entry).await; skills.append(&mut part); } + #[cfg(feature = "product-full")] + { + let existing_paths = skills + .iter() + .map(canonical_candidate_path) + .collect::>(); + let roots = opencode_configured_skill_roots(workspace_root); + let mut configured = Self::scan_configured_opencode_candidates(roots).await; + configured + .retain(|candidate| !existing_paths.contains(&canonical_candidate_path(candidate))); + skills = Self::merge_configured_opencode_candidates( + skills, + configured, + workspace_root.is_some(), + ); + } skills } + #[cfg(feature = "product-full")] + fn merge_configured_opencode_candidates( + mut standard: Vec, + mut configured: Vec, + has_workspace: bool, + ) -> Vec { + if configured.is_empty() { + return standard; + } + + let has_project = configured + .iter() + .any(|candidate| candidate.info.level == SkillLocation::Project); + let has_user = configured + .iter() + .any(|candidate| candidate.info.level == SkillLocation::User); + let project_anchor = PROJECT_SKILL_ROOTS + .iter() + .position(|root| root.source_id == "opencode") + .expect("OpenCode project Skill root is registered"); + let user_anchor = has_workspace + .then_some(PROJECT_SKILL_ROOTS.len()) + .unwrap_or_default() + .saturating_add( + USER_HOME_SKILL_ROOTS + .iter() + .position(|root| root.source_id == "opencode") + .expect("OpenCode user Skill root is registered"), + ); + + for candidate in &mut standard { + let original_priority = candidate.priority; + let project_shift = (has_project && original_priority >= project_anchor) + .then_some(OPENCODE_CONFIGURED_PRIORITY_BAND) + .unwrap_or_default(); + let user_shift = (has_user && original_priority >= user_anchor) + .then_some(OPENCODE_CONFIGURED_PRIORITY_BAND) + .unwrap_or_default(); + candidate.priority = original_priority + .saturating_add(project_shift) + .saturating_add(user_shift); + } + for candidate in &mut configured { + let anchor = match candidate.info.level { + SkillLocation::Project => project_anchor, + SkillLocation::User => user_anchor.saturating_add( + has_project + .then_some(OPENCODE_CONFIGURED_PRIORITY_BAND) + .unwrap_or_default(), + ), + }; + candidate.priority = candidate.priority.saturating_add(anchor); + } + standard.extend(configured); + standard + } + + #[cfg(feature = "product-full")] + async fn scan_configured_opencode_candidates( + roots: Vec, + ) -> Vec { + let mut roots = roots; + roots.sort_by_key(|root| root.precedence); + let roots = roots + .into_iter() + .rev() + .take(MAX_OPENCODE_CONFIGURED_SKILL_ROOTS) + .collect::>() + .into_iter() + .rev() + .collect::>(); + let project_root_count = roots + .iter() + .filter(|root| { + matches!( + root.scope, + bitfun_product_domains::external_sources::ExternalSourceScope::Project + | bitfun_product_domains::external_sources::ExternalSourceScope::WorkspaceLocal + ) + }) + .count(); + let user_root_count = roots + .iter() + .filter(|root| { + root.scope + == bitfun_product_domains::external_sources::ExternalSourceScope::UserGlobal + }) + .count(); + let mut project_root_index = 0usize; + let mut user_root_index = 0usize; + let mut candidates = Vec::new(); + + for root in roots { + let root_path = root.path.clone(); + let files = match tokio::task::spawn_blocking(move || { + collect_bounded_regular_files( + &root_path, + BoundedDirectoryWalkLimits { + max_depth: 16, + max_entries: 4096, + max_directories: 2048, + max_files: MAX_OPENCODE_CONFIGURED_SKILLS_PER_ROOT, + }, + |path| path.file_name().and_then(|name| name.to_str()) == Some("SKILL.md"), + ) + }) + .await + { + Ok(Ok(files)) => files, + Ok(Err(error)) => { + warn!( + "Skipping configured OpenCode skill root {}: {}", + root.path.display(), + error + ); + continue; + } + Err(error) => { + warn!( + "Configured OpenCode skill scan failed for {}: {}", + root.path.display(), + error + ); + continue; + } + }; + let file_count = files.len(); + for (file_index, skill_md_path) in files.into_iter().enumerate() { + let Some(skill_dir) = skill_md_path.parent() else { + continue; + }; + let Some(dir_name) = normalize_local_skill_dir_name(skill_dir) else { + continue; + }; + let canonical_skill_dir = + dunce::canonicalize(skill_dir).unwrap_or_else(|_| skill_dir.to_path_buf()); + let source_slot = configured_opencode_source_slot(&canonical_skill_dir); + let read_path = skill_md_path.clone(); + let content = match tokio::task::spawn_blocking(move || { + read_bounded_text(&read_path, MAX_OPENCODE_CONFIGURED_SKILL_BYTES) + }) + .await + { + Ok(Ok(BoundedTextRead::Content(content))) => content, + Ok(Ok(BoundedTextRead::TooLarge)) => { + warn!( + "Skipping configured OpenCode skill file above the {} byte limit: {}", + MAX_OPENCODE_CONFIGURED_SKILL_BYTES, + skill_md_path.display() + ); + continue; + } + Ok(Ok(BoundedTextRead::InvalidUtf8)) => { + warn!( + "Skipping configured OpenCode skill file that is not valid UTF-8: {}", + skill_md_path.display() + ); + continue; + } + Ok(Err(error)) => { + debug!("Failed to read {}: {}", skill_md_path.display(), error); + continue; + } + Err(error) => { + warn!( + "Configured OpenCode skill read failed for {}: {}", + skill_md_path.display(), + error + ); + continue; + } + }; + let location = match root.scope { + bitfun_product_domains::external_sources::ExternalSourceScope::UserGlobal => { + SkillLocation::User + } + bitfun_product_domains::external_sources::ExternalSourceScope::Project + | bitfun_product_domains::external_sources::ExternalSourceScope::WorkspaceLocal => { + SkillLocation::Project + } + _ => continue, + }; + let (scope_root_count, scope_root_index) = match location { + SkillLocation::Project => (project_root_count, project_root_index), + SkillLocation::User => (user_root_count, user_root_index), + }; + let mut skill_data = match Self::parse_skill_markdown( + canonical_skill_dir.to_string_lossy().to_string(), + &content, + location, + false, + &source_slot, + ) { + Ok(skill_data) => skill_data, + Err(error) => { + error!( + "Failed to parse configured OpenCode SKILL.md in {}: {}", + canonical_skill_dir.display(), + error + ); + continue; + } + }; + Self::apply_configured_opencode_policy( + &mut skill_data, + &canonical_skill_dir, + &source_slot, + ) + .await; + skill_data.dir_name = dir_name; + let root_rank = scope_root_count.saturating_sub(scope_root_index + 1); + let file_rank = file_count.saturating_sub(file_index + 1); + let priority = root_rank + .saturating_mul(MAX_OPENCODE_CONFIGURED_SKILLS_PER_ROOT) + .saturating_add(file_rank); + let key_prefix = match location { + SkillLocation::User => USER_SKILL_KEY_PREFIX, + SkillLocation::Project => PROJECT_SKILL_KEY_PREFIX, + }; + candidates.push(SkillCandidate::from_data( + skill_data, + &source_slot, + "opencode", + "OpenCode", + key_prefix, + priority, + false, + )); + } + match root.scope { + bitfun_product_domains::external_sources::ExternalSourceScope::UserGlobal => { + user_root_index = user_root_index.saturating_add(1); + } + bitfun_product_domains::external_sources::ExternalSourceScope::Project + | bitfun_product_domains::external_sources::ExternalSourceScope::WorkspaceLocal => { + project_root_index = project_root_index.saturating_add(1); + } + _ => {} + } + } + candidates.sort_by_key(|candidate| candidate.priority); + let mut seen_paths = HashSet::new(); + candidates.retain(|candidate| seen_paths.insert(candidate.info.path.clone())); + sort_skill_candidates_by_dir(candidates) + } + async fn scan_remote_project_skills( fs: &dyn WorkspaceFileSystem, remote_root: &str, @@ -814,10 +1217,7 @@ impl SkillRegistry { ) .await?; - let skill_md_path = PathBuf::from(&info.path).join("SKILL.md"); - let content = fs::read_to_string(&skill_md_path) - .await - .map_err(|error| BitFunError::tool(format!("Failed to read skill file: {}", error)))?; + let content = Self::read_local_skill_markdown(&info).await?; let mut data = Self::parse_skill_markdown( info.path.clone(), @@ -856,10 +1256,7 @@ impl SkillRegistry { )) })?; - let skill_md_path = PathBuf::from(&info.path).join("SKILL.md"); - let content = fs::read_to_string(&skill_md_path) - .await - .map_err(|error| BitFunError::tool(format!("Failed to read skill file: {}", error)))?; + let content = Self::read_local_skill_markdown(&info).await?; let mut data = Self::parse_skill_markdown( info.path.clone(), @@ -975,12 +1372,7 @@ impl SkillRegistry { remote_fs: &dyn WorkspaceFileSystem, ) -> BitFunResult { match info.level { - SkillLocation::User => { - let skill_md_path = PathBuf::from(&info.path).join("SKILL.md"); - fs::read_to_string(&skill_md_path).await.map_err(|error| { - BitFunError::tool(format!("Failed to read skill file: {}", error)) - }) - } + SkillLocation::User => Self::read_local_skill_markdown(info).await, SkillLocation::Project => { let skill_md_path = format!("{}/SKILL.md", info.path.trim_end_matches('/')); remote_fs @@ -993,3 +1385,337 @@ impl SkillRegistry { } } } + +#[cfg(all(test, feature = "product-full"))] +mod opencode_configured_skill_tests { + use super::{SkillRegistry, SkillRootEntry}; + use crate::external_sources::LocalConfiguredSkillRootContribution; + use bitfun_agent_runtime::skills::{resolve_visible_skills, SkillLocation}; + use bitfun_product_domains::external_sources::ExternalSourceScope; + use std::fs; + use std::path::{Path, PathBuf}; + + fn write(path: impl AsRef, content: &str) { + let path = path.as_ref(); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, content).unwrap(); + } + + fn skill(name: &str) -> String { + format!("---\nname: {name}\ndescription: {name} skill\n---\nRun {name}.\n") + } + + fn configured_root( + path: PathBuf, + scope: ExternalSourceScope, + precedence: usize, + ) -> LocalConfiguredSkillRootContribution { + LocalConfiguredSkillRootContribution { + path: dunce::canonicalize(path).unwrap(), + scope, + precedence, + } + } + + #[tokio::test] + async fn configured_roots_are_recursive_and_nested_same_named_dirs_keep_unique_keys() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + fs::create_dir_all(project.join(".git")).unwrap(); + write( + project.join("custom-skills/a/foo/SKILL.md"), + &skill("first"), + ); + write( + project.join("custom-skills/b/foo/SKILL.md"), + &skill("second"), + ); + let roots = vec![configured_root( + project.join("custom-skills"), + ExternalSourceScope::Project, + 0, + )]; + + let candidates = SkillRegistry::scan_configured_opencode_candidates(roots).await; + + assert_eq!(candidates.len(), 2); + assert_ne!(candidates[0].info.key, candidates[1].info.key); + assert_ne!( + candidates[0].info.source_slot, + candidates[1].info.source_slot + ); + assert!(candidates + .iter() + .all(|candidate| candidate.info.source_slot.starts_with("config.opencode."))); + } + + #[tokio::test] + async fn later_configured_root_overrides_standard_opencode_skill() { + let temp = tempfile::tempdir().unwrap(); + let home = temp.path().join("home"); + let project = temp.path().join("project"); + fs::create_dir_all(project.join(".git")).unwrap(); + write(home.join("first-skills/review/SKILL.md"), &skill("review")); + write( + project.join("later-skills/review/SKILL.md"), + &skill("review"), + ); + write( + project.join(".opencode/skills/review/SKILL.md"), + &skill("review"), + ); + let roots = vec![ + configured_root( + home.join("first-skills"), + ExternalSourceScope::UserGlobal, + 0, + ), + configured_root( + project.join("later-skills"), + ExternalSourceScope::Project, + 1, + ), + ]; + + let standard = SkillRegistry::scan_skills_in_dir(&SkillRootEntry { + path: project.join(".opencode/skills"), + level: SkillLocation::Project, + slot: "opencode", + source_id: "opencode", + source_label: "OpenCode", + priority: super::PROJECT_SKILL_ROOTS + .iter() + .position(|root| root.source_id == "opencode") + .unwrap(), + is_builtin: false, + }) + .await; + let configured = SkillRegistry::scan_configured_opencode_candidates(roots).await; + let candidates = + SkillRegistry::merge_configured_opencode_candidates(standard, configured, true); + let resolved = resolve_visible_skills(candidates); + + assert_eq!(resolved.len(), 1); + assert!(resolved[0].path.contains("later-skills")); + } + + #[tokio::test] + async fn configured_opencode_roots_do_not_reorder_earlier_standard_ecosystems() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + write( + project.join(".bitfun/skills/review/SKILL.md"), + &skill("review"), + ); + write(project.join("custom/review/SKILL.md"), &skill("review")); + let standard = SkillRegistry::scan_skills_in_dir(&SkillRootEntry { + path: project.join(".bitfun/skills"), + level: SkillLocation::Project, + slot: "bitfun", + source_id: "bitfun", + source_label: "BitFun", + priority: 0, + is_builtin: false, + }) + .await; + let configured = SkillRegistry::scan_configured_opencode_candidates(vec![configured_root( + project.join("custom"), + ExternalSourceScope::Project, + 0, + )]) + .await; + + let resolved = resolve_visible_skills(SkillRegistry::merge_configured_opencode_candidates( + standard, configured, true, + )); + + assert_eq!(resolved.len(), 1); + assert!(resolved[0].path.contains(".bitfun")); + } + + #[tokio::test] + async fn project_configured_band_does_not_shift_user_ecosystem_order_twice() { + let temp = tempfile::tempdir().unwrap(); + let home = temp.path().join("home"); + let project = temp.path().join("project"); + write( + home.join(".claude/skills/review/SKILL.md"), + &skill("review"), + ); + write(home.join("configured/review/SKILL.md"), &skill("review")); + write(project.join("configured/other/SKILL.md"), &skill("other")); + let standard = SkillRegistry::scan_skills_in_dir(&SkillRootEntry { + path: home.join(".claude/skills"), + level: SkillLocation::User, + slot: "home.claude", + source_id: "claude-code", + source_label: "Claude Code", + priority: super::PROJECT_SKILL_ROOTS.len(), + is_builtin: false, + }) + .await; + let configured = SkillRegistry::scan_configured_opencode_candidates(vec![ + configured_root(home.join("configured"), ExternalSourceScope::UserGlobal, 0), + configured_root(project.join("configured"), ExternalSourceScope::Project, 1), + ]) + .await; + + let resolved = resolve_visible_skills(SkillRegistry::merge_configured_opencode_candidates( + standard, configured, true, + )); + let review = resolved + .iter() + .find(|skill| skill.name == "review") + .unwrap(); + + assert!(review.path.contains(".claude")); + } + + #[tokio::test] + async fn overlapping_configured_roots_publish_each_canonical_skill_once() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + fs::create_dir_all(project.join(".git")).unwrap(); + write( + project.join("skills/nested/review/SKILL.md"), + &skill("review"), + ); + let roots = vec![ + configured_root(project.join("skills"), ExternalSourceScope::Project, 0), + configured_root( + project.join("skills/nested"), + ExternalSourceScope::Project, + 1, + ), + ]; + + let candidates = SkillRegistry::scan_configured_opencode_candidates(roots).await; + + assert_eq!(candidates.len(), 1); + assert_eq!(candidates[0].info.name, "review"); + } + + #[tokio::test] + async fn oversized_configured_skill_does_not_hide_other_valid_skills() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + fs::create_dir_all(project.join(".git")).unwrap(); + write(project.join("skills/valid/SKILL.md"), &skill("valid")); + write( + project.join("skills/oversized/SKILL.md"), + &"x".repeat(super::MAX_OPENCODE_CONFIGURED_SKILL_BYTES + 1), + ); + let roots = vec![configured_root( + project.join("skills"), + ExternalSourceScope::Project, + 0, + )]; + + let candidates = SkillRegistry::scan_configured_opencode_candidates(roots).await; + + assert_eq!(candidates.len(), 1); + assert_eq!(candidates[0].info.name, "valid"); + } + + #[tokio::test] + async fn configured_skill_load_rechecks_the_bounded_file_contract() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + let skill_path = project.join("skills/review/SKILL.md"); + write(&skill_path, &skill("review")); + let candidates = SkillRegistry::scan_configured_opencode_candidates(vec![configured_root( + project.join("skills"), + ExternalSourceScope::Project, + 0, + )]) + .await; + assert_eq!(candidates.len(), 1); + fs::write( + &skill_path, + "x".repeat(super::MAX_OPENCODE_CONFIGURED_SKILL_BYTES + 1), + ) + .unwrap(); + + let error = SkillRegistry::read_local_skill_markdown(&candidates[0].info) + .await + .unwrap_err(); + + assert!(error.to_string().contains("262144 byte limit")); + } + + #[tokio::test] + async fn configured_skill_load_rejects_a_replaced_root_link() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + let skill_dir = project.join("skills/review"); + write(skill_dir.join("SKILL.md"), &skill("review")); + let candidates = SkillRegistry::scan_configured_opencode_candidates(vec![configured_root( + project.join("skills"), + ExternalSourceScope::Project, + 0, + )]) + .await; + assert_eq!(candidates.len(), 1); + let moved = temp.path().join("moved-review"); + fs::rename(&skill_dir, &moved).unwrap(); + if !create_dir_symlink(&moved, &skill_dir) { + return; + } + + let error = SkillRegistry::read_local_skill_markdown(&candidates[0].info) + .await + .unwrap_err(); + + assert!(error.to_string().contains("must not be a symlink")); + } + + #[tokio::test] + async fn configured_skill_policy_is_bounded_and_does_not_follow_directory_links() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path().join("project"); + let skill_dir = project.join("skills/review"); + write(skill_dir.join("SKILL.md"), &skill("review")); + write( + skill_dir.join("agents/openai.yaml"), + &format!( + "policy:\n allow_implicit_invocation: false\n{}", + " ".repeat(super::MAX_OPENCODE_CONFIGURED_POLICY_BYTES) + ), + ); + let roots = vec![configured_root( + project.join("skills"), + ExternalSourceScope::Project, + 0, + )]; + + let oversized = SkillRegistry::scan_configured_opencode_candidates(roots.clone()).await; + + assert_eq!(oversized.len(), 1); + assert!(oversized[0].info.allow_implicit_invocation); + + fs::remove_dir_all(skill_dir.join("agents")).unwrap(); + let outside = temp.path().join("outside-agents"); + write( + outside.join("openai.yaml"), + "policy:\n allow_implicit_invocation: false\n", + ); + if !create_dir_symlink(&outside, &skill_dir.join("agents")) { + return; + } + + let linked = SkillRegistry::scan_configured_opencode_candidates(roots).await; + + assert_eq!(linked.len(), 1); + assert!(linked[0].info.allow_implicit_invocation); + } + + #[cfg(unix)] + fn create_dir_symlink(target: &Path, link: &Path) -> bool { + std::os::unix::fs::symlink(target, link).is_ok() + } + + #[cfg(windows)] + fn create_dir_symlink(target: &Path, link: &Path) -> bool { + std::os::windows::fs::symlink_dir(target, link).is_ok() + } +} diff --git a/src/crates/assembly/core/src/external_sources.rs b/src/crates/assembly/core/src/external_sources.rs index 0b2870eef..6b0b6b979 100644 --- a/src/crates/assembly/core/src/external_sources.rs +++ b/src/crates/assembly/core/src/external_sources.rs @@ -62,11 +62,13 @@ use bitfun_external_sources::{ ExternalToolDiscoveryResult, }; use bitfun_opencode_adapter::{ - OpenCodeCommandProvider, OpenCodeMcpProvider, OpenCodeSubagentProvider, OpenCodeToolProvider, + OpenCodeCommandProvider, OpenCodeMcpProvider, OpenCodeSkillRootProvider, + OpenCodeSubagentProvider, OpenCodeToolProvider, }; #[cfg(test)] use bitfun_opencode_adapter::{ - OpenCodeCommandProviderOptions, OpenCodeMcpProviderOptions, OpenCodeSubagentProviderOptions, + OpenCodeCommandProviderOptions, OpenCodeMcpProviderOptions, OpenCodeSkillRootProviderOptions, + OpenCodeSubagentProviderOptions, }; use bitfun_product_domains::external_integration_policy::{ external_integration_policy_snapshot, incompatible_external_integration_policy_snapshot, @@ -76,11 +78,12 @@ use bitfun_product_domains::external_integration_policy::{ }; use bitfun_product_domains::external_sources::{ ExecutionDomainId, ExternalMcpRevisionKey, ExternalMcpSourceProvider, ExternalMcpStaticStatus, - ExternalSourceContext, ExternalSourceScope, ExternalToolSourceProvider, + ExternalSourceContext, ExternalSourceScope, ExternalToolSourceProvider, PromptCommandExpansion, PromptCommandSourceProvider, }; use bitfun_product_domains::external_subagents::ExternalSubagentSourceProvider; use bitfun_services_core::json_store::JsonFileStore; +use bitfun_services_core::workspace_text::read_workspace_relative_text_bounded; use bitfun_services_integrations::file_watch::{FileWatchService, FileWatcherConfig}; use dashmap::{mapref::entry::Entry, DashMap}; use futures::future::join_all; @@ -104,6 +107,126 @@ pub const EXTERNAL_CAPABILITY_TOOL: &str = "tool"; pub const EXTERNAL_CAPABILITY_SUBAGENT: &str = "subagent"; pub const EXTERNAL_CAPABILITY_MCP: &str = "mcp"; const EXTERNAL_ADAPTER_CONTRACT_MAJOR: u32 = 1; +const MAX_PROMPT_COMMAND_FILE_REFERENCES: usize = 8; +const MAX_PROMPT_COMMAND_FILE_BYTES: usize = 64 * 1024; +const MAX_PROMPT_COMMAND_TOTAL_FILE_BYTES: usize = 128 * 1024; +const MAX_EXPANDED_PROMPT_COMMAND_BYTES: usize = 1024 * 1024; + +#[derive(Debug, Clone)] +pub(crate) struct LocalConfiguredSkillRootContribution { + pub path: PathBuf, + pub scope: ExternalSourceScope, + pub precedence: usize, +} + +pub(crate) fn opencode_configured_skill_roots( + workspace_root: Option<&Path>, +) -> Vec { + opencode_configured_skill_roots_with_provider( + workspace_root, + &OpenCodeSkillRootProvider::default(), + ) +} + +fn opencode_configured_skill_roots_with_provider( + workspace_root: Option<&Path>, + provider: &OpenCodeSkillRootProvider, +) -> Vec { + provider + .discover(workspace_root) + .into_iter() + .map(|root| LocalConfiguredSkillRootContribution { + path: root.path, + scope: root.scope, + precedence: root.precedence, + }) + .collect() +} + +async fn finalize_prompt_command_expansion( + workspace_root: Option<&Path>, + expansion: PromptCommandExpansion, +) -> Result { + if expansion.content.len() > MAX_EXPANDED_PROMPT_COMMAND_BYTES { + return Err(format!( + "expanded external prompt command exceeds the {MAX_EXPANDED_PROMPT_COMMAND_BYTES} byte limit" + )); + } + if expansion.workspace_file_references.is_empty() { + return Ok(ExpandedPromptCommand { + content: expansion.content, + }); + } + + let workspace_root = workspace_root.ok_or_else(|| { + "external prompt command file reference expansion requires a local workspace".to_string() + })?; + let mut seen = BTreeSet::new(); + let references = expansion + .workspace_file_references + .into_iter() + .filter(|reference| seen.insert(reference.clone())) + .collect::>(); + if references.len() > MAX_PROMPT_COMMAND_FILE_REFERENCES { + return Err(format!( + "external prompt commands may reference at most {MAX_PROMPT_COMMAND_FILE_REFERENCES} workspace files" + )); + } + + let mut total_file_bytes = 0usize; + let mut files = Vec::with_capacity(references.len()); + for reference in references { + let file = read_workspace_relative_text_bounded( + workspace_root, + &reference, + MAX_PROMPT_COMMAND_FILE_BYTES, + ) + .await + .map_err(|error| { + format!("failed to read referenced workspace file '{reference}': {error}") + })?; + total_file_bytes = total_file_bytes + .checked_add(file.byte_len) + .ok_or_else(|| "referenced workspace files exceed the total byte limit".to_string())?; + if total_file_bytes > MAX_PROMPT_COMMAND_TOTAL_FILE_BYTES { + return Err(format!( + "referenced workspace files exceed the {MAX_PROMPT_COMMAND_TOTAL_FILE_BYTES} byte total limit" + )); + } + files.push(file); + } + + let mut content = expansion.content; + content.push_str("\n\n## Referenced workspace files"); + for file in files { + let fence = markdown_code_fence(&file.content); + content.push_str("\n\n### `"); + content.push_str(&file.relative_path); + content.push_str("`\n\n"); + content.push_str(&fence); + content.push_str("text\n"); + content.push_str(&file.content); + if !file.content.ends_with('\n') { + content.push('\n'); + } + content.push_str(&fence); + } + if content.len() > MAX_EXPANDED_PROMPT_COMMAND_BYTES { + return Err(format!( + "expanded external prompt command exceeds the {MAX_EXPANDED_PROMPT_COMMAND_BYTES} byte limit" + )); + } + Ok(ExpandedPromptCommand { content }) +} + +fn markdown_code_fence(content: &str) -> String { + let longest_run = content + .split(|character| character != '`') + .map(str::len) + .max() + .unwrap_or_default(); + "`".repeat(longest_run.saturating_add(1).max(3)) +} fn external_capability_descriptor( capability_id: &str, @@ -2780,7 +2903,7 @@ impl WorkspaceExternalSourceService { let arguments = arguments.to_string(); let expected_candidate_id = expected_candidate_id.map(str::to_string); let expected_content_version = expected_content_version.map(str::to_string); - tokio::task::spawn_blocking(move || { + let expansion = tokio::task::spawn_blocking(move || { lock_coordinator(&coordinator) .expand_command_guarded( &name, @@ -2791,7 +2914,8 @@ impl WorkspaceExternalSourceService { .map_err(|error| error.to_string()) }) .await - .map_err(|error| format!("external command expansion task failed: {error}"))? + .map_err(|error| format!("external command expansion task failed: {error}"))??; + finalize_prompt_command_expansion(self.workspace_root.as_deref(), expansion).await } async fn start_watching(self: &Arc) { @@ -4786,11 +4910,15 @@ fn project_native_prompt_command_conflicts( .collect::>() }; let Some((_, _, source)) = external.first() else { - reconfirmations.extend(native.iter().filter(|&command| conflicted_candidate_ids - .contains(&command.candidate_id)).map(|command| NativePromptCommandReconfirmationProjection { + reconfirmations.extend( + native + .iter() + .filter(|&command| conflicted_candidate_ids.contains(&command.candidate_id)) + .map(|command| NativePromptCommandReconfirmationProjection { command_name: command_name.clone(), native_candidate_id: command.candidate_id.clone(), - })); + }), + ); continue; }; let execution_domain = snapshot @@ -6385,9 +6513,10 @@ mod tests { &self, command: &PromptCommandDefinition, _arguments: &str, - ) -> Result { - Ok(ExpandedPromptCommand { + ) -> Result { + Ok(PromptCommandExpansion { content: command.template.clone(), + workspace_file_references: Vec::new(), }) } @@ -6465,6 +6594,188 @@ mod tests { }) } + #[test] + fn composition_projects_opencode_configured_skill_roots_without_leaking_adapter_types() { + let temp = tempfile::tempdir().unwrap(); + let home = temp.path().join("home"); + let user_config = home.join(".config/opencode"); + let project = temp.path().join("project"); + std::fs::create_dir_all(project.join(".git")).unwrap(); + std::fs::create_dir_all(project.join("shared-skills")).unwrap(); + std::fs::create_dir_all(&user_config).unwrap(); + std::fs::write( + project.join("opencode.json"), + r#"{"skills":["shared-skills"]}"#, + ) + .unwrap(); + let provider = OpenCodeSkillRootProvider::new(OpenCodeSkillRootProviderOptions { + command: OpenCodeCommandProviderOptions { + user_config_dir: user_config, + legacy_user_config_dir: Some(home.join(".opencode")), + explicit_config_file: None, + explicit_config_dir: None, + project_config_enabled: true, + }, + home_dir: Some(home), + }); + + let roots = opencode_configured_skill_roots_with_provider(Some(&project), &provider); + + assert_eq!(roots.len(), 1); + assert_eq!( + roots[0].path, + dunce::canonicalize(project.join("shared-skills")).unwrap() + ); + assert_eq!(roots[0].scope, ExternalSourceScope::Project); + } + + #[tokio::test] + async fn finalizes_prompt_command_files_without_changing_plain_commands() { + let workspace = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(workspace.path().join("src")).unwrap(); + std::fs::write( + workspace.path().join("src/lib.rs"), + "pub fn answer() -> u8 { 42 }\n``` embedded", + ) + .unwrap(); + std::fs::write(workspace.path().join("README.md"), "Read me").unwrap(); + + let plain = finalize_prompt_command_expansion( + None, + PromptCommandExpansion { + content: "plain prompt".to_string(), + workspace_file_references: Vec::new(), + }, + ) + .await + .unwrap(); + assert_eq!(plain.content, "plain prompt"); + + let expanded = finalize_prompt_command_expansion( + Some(workspace.path()), + PromptCommandExpansion { + content: "Review @src/lib.rs and @README.md".to_string(), + workspace_file_references: vec![ + "src/lib.rs".to_string(), + "README.md".to_string(), + "src/lib.rs".to_string(), + ], + }, + ) + .await + .unwrap(); + + assert!(expanded + .content + .starts_with("Review @src/lib.rs and @README.md")); + assert_eq!(expanded.content.matches("### `src/lib.rs`").count(), 1); + assert_eq!(expanded.content.matches("### `README.md`").count(), 1); + assert!(expanded.content.contains("````text\npub fn answer()")); + assert!(expanded.content.contains("\n````")); + } + + #[tokio::test] + async fn prompt_command_file_limits_and_failures_are_atomic() { + let workspace = tempfile::tempdir().unwrap(); + std::fs::write( + workspace.path().join("large.txt"), + vec![b'x'; MAX_PROMPT_COMMAND_FILE_BYTES + 1], + ) + .unwrap(); + for index in 0..=MAX_PROMPT_COMMAND_FILE_REFERENCES { + std::fs::write(workspace.path().join(format!("{index}.txt")), "x").unwrap(); + } + for index in 0..3 { + std::fs::write( + workspace.path().join(format!("total-{index}.txt")), + vec![b'x'; 48 * 1024], + ) + .unwrap(); + } + + let without_workspace = finalize_prompt_command_expansion( + None, + PromptCommandExpansion { + content: "prompt".to_string(), + workspace_file_references: vec!["0.txt".to_string()], + }, + ) + .await + .unwrap_err(); + assert!(without_workspace.contains("requires a local workspace")); + + let too_many = finalize_prompt_command_expansion( + Some(workspace.path()), + PromptCommandExpansion { + content: "prompt".to_string(), + workspace_file_references: (0..=MAX_PROMPT_COMMAND_FILE_REFERENCES) + .map(|index| format!("{index}.txt")) + .collect(), + }, + ) + .await + .unwrap_err(); + assert!(too_many.contains("at most 8")); + + let oversized = finalize_prompt_command_expansion( + Some(workspace.path()), + PromptCommandExpansion { + content: "prompt".to_string(), + workspace_file_references: vec!["large.txt".to_string()], + }, + ) + .await + .unwrap_err(); + assert!(oversized.contains("65536 byte limit")); + + let total_oversized = finalize_prompt_command_expansion( + Some(workspace.path()), + PromptCommandExpansion { + content: "prompt".to_string(), + workspace_file_references: (0..3) + .map(|index| format!("total-{index}.txt")) + .collect(), + }, + ) + .await + .unwrap_err(); + assert!(total_oversized.contains("131072 byte total limit")); + + let final_oversized = finalize_prompt_command_expansion( + Some(workspace.path()), + PromptCommandExpansion { + content: "x".repeat(MAX_EXPANDED_PROMPT_COMMAND_BYTES), + workspace_file_references: vec!["0.txt".to_string()], + }, + ) + .await + .unwrap_err(); + assert!(final_oversized.contains("1048576 byte limit")); + + let plain_oversized = finalize_prompt_command_expansion( + None, + PromptCommandExpansion { + content: "x".repeat(MAX_EXPANDED_PROMPT_COMMAND_BYTES + 1), + workspace_file_references: Vec::new(), + }, + ) + .await + .unwrap_err(); + assert!(plain_oversized.contains("1048576 byte limit")); + + let one_bad_file = finalize_prompt_command_expansion( + Some(workspace.path()), + PromptCommandExpansion { + content: "must not be returned partially".to_string(), + workspace_file_references: vec!["0.txt".to_string(), "missing.txt".to_string()], + }, + ) + .await + .unwrap_err(); + assert!(one_bad_file.contains("missing.txt")); + assert!(!one_bad_file.contains("must not be returned partially")); + } + #[test] fn surface_snapshot_projects_control_and_legacy_catalog_from_one_generation() { let service = test_service(Vec::new()); diff --git a/src/crates/assembly/external-sources/src/lib.rs b/src/crates/assembly/external-sources/src/lib.rs index e53bcdce8..ea8f50081 100644 --- a/src/crates/assembly/external-sources/src/lib.rs +++ b/src/crates/assembly/external-sources/src/lib.rs @@ -31,13 +31,13 @@ pub use tool::{ }; use bitfun_product_domains::external_sources::{ - prompt_command_conflict_key, EcosystemId, ExpandedPromptCommand, ExternalSourceCatalogEntry, + prompt_command_conflict_key, EcosystemId, ExternalSourceCatalogEntry, ExternalSourceCatalogSnapshot, ExternalSourceContext, ExternalSourceDiagnostic, ExternalSourceHealth, ExternalSourceLifecycleState, ExternalSourceProviderError, ExternalSourceRecord, ExternalWatchRoot, PromptCommandAvailability, PromptCommandCatalogEntry, PromptCommandConflict, PromptCommandConflictCandidate, PromptCommandDefinition, - PromptCommandProviderIdentity, PromptCommandProviderSnapshot, PromptCommandSourceProvider, - ProviderId, SourceKey, + PromptCommandExpansion, PromptCommandProviderIdentity, PromptCommandProviderSnapshot, + PromptCommandSourceProvider, ProviderId, SourceKey, }; use std::collections::{BTreeMap, BTreeSet}; use std::fmt; @@ -458,7 +458,7 @@ impl ExternalSourceCoordinator { &self, name: &str, arguments: &str, - ) -> Result { + ) -> Result { self.expand_command_guarded(name, arguments, None, None) } @@ -468,7 +468,7 @@ impl ExternalSourceCoordinator { arguments: &str, expected_candidate_id: Option<&str>, expected_content_version: Option<&str>, - ) -> Result { + ) -> Result { let command = self .snapshot .commands diff --git a/src/crates/assembly/external-sources/tests/coordinator_contracts.rs b/src/crates/assembly/external-sources/tests/coordinator_contracts.rs index 78deb5cd3..c808c2fce 100644 --- a/src/crates/assembly/external-sources/tests/coordinator_contracts.rs +++ b/src/crates/assembly/external-sources/tests/coordinator_contracts.rs @@ -1,9 +1,9 @@ use bitfun_external_sources::ExternalSourceCoordinator; use bitfun_product_domains::external_sources::{ - EcosystemId, ExecutionDomainId, ExpandedPromptCommand, ExternalSourceContext, - ExternalSourceHealth, ExternalSourceLifecycleState, ExternalSourceProviderError, - ExternalSourceRecord, ExternalSourceScope, ExternalWatchRoot, PromptCommandAvailability, - PromptCommandDefinition, PromptCommandProviderIdentity, PromptCommandProviderSnapshot, + EcosystemId, ExecutionDomainId, ExternalSourceContext, ExternalSourceHealth, + ExternalSourceLifecycleState, ExternalSourceProviderError, ExternalSourceRecord, + ExternalSourceScope, ExternalWatchRoot, PromptCommandAvailability, PromptCommandDefinition, + PromptCommandExpansion, PromptCommandProviderIdentity, PromptCommandProviderSnapshot, PromptCommandSourceProvider, SourceKey, SourceQualifiedCommandId, }; use std::path::PathBuf; @@ -113,9 +113,10 @@ impl PromptCommandSourceProvider for FakeProvider { &self, command: &PromptCommandDefinition, arguments: &str, - ) -> Result { - Ok(ExpandedPromptCommand { + ) -> Result { + Ok(PromptCommandExpansion { content: command.template.replace("$ARGUMENTS", arguments), + workspace_file_references: vec!["src/lib.rs".to_string()], }) } @@ -394,6 +395,7 @@ fn suppression_survives_refresh_and_expansion_dispatches_by_provider_identity() .expand_command("review", "this change") .expect("expand active command"); assert_eq!(expanded.content, "first: this change"); + assert_eq!(expanded.workspace_file_references, ["src/lib.rs"]); coordinator .set_source_enabled(&second_key, true) diff --git a/src/crates/contracts/product-domains/src/external_sources.rs b/src/crates/contracts/product-domains/src/external_sources.rs index e7712b222..851339196 100644 --- a/src/crates/contracts/product-domains/src/external_sources.rs +++ b/src/crates/contracts/product-domains/src/external_sources.rs @@ -1505,6 +1505,14 @@ pub struct ExpandedPromptCommand { pub content: String, } +/// Provider-prepared command content plus the narrow set of local workspace +/// files that Product Assembly must resolve before sending the final prompt. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PromptCommandExpansion { + pub content: String, + pub workspace_file_references: Vec, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct PromptCommandProviderIdentity { @@ -1684,7 +1692,7 @@ pub trait PromptCommandSourceProvider: Send + Sync { &self, command: &PromptCommandDefinition, arguments: &str, - ) -> Result; + ) -> Result; /// Resolves same-ecosystem overlays after product suppression is applied. /// The full snapshot is supplied so a provider can preserve native source diff --git a/src/crates/contracts/product-domains/tests/external_source_contracts.rs b/src/crates/contracts/product-domains/tests/external_source_contracts.rs index ae8e69169..ad101c8d1 100644 --- a/src/crates/contracts/product-domains/tests/external_source_contracts.rs +++ b/src/crates/contracts/product-domains/tests/external_source_contracts.rs @@ -27,9 +27,10 @@ use bitfun_product_domains::external_sources::{ ExternalToolStaticStatus, ExternalWatchRoot, NativePromptCommandDescriptor, PreparedExternalMcpImportServer, PreparedExternalMcpImportTransport, PreparedExternalMcpServer, PreparedExternalMcpTransport, PromptCommandAvailability, PromptCommandCatalogEntry, - PromptCommandDefinition, PromptCommandProviderIdentity, PromptCommandProviderSnapshot, - PromptCommandSourceProvider, SecretValue, SourceKey, SourceQualifiedCommandId, - SourceQualifiedMcpServerId, SourceQualifiedToolId, SourceQualifiedToolTargetId, + PromptCommandDefinition, PromptCommandExpansion, PromptCommandProviderIdentity, + PromptCommandProviderSnapshot, PromptCommandSourceProvider, SecretValue, SourceKey, + SourceQualifiedCommandId, SourceQualifiedMcpServerId, SourceQualifiedToolId, + SourceQualifiedToolTargetId, }; use bitfun_product_domains::external_subagents::{ external_subagent_approval_key, external_subagent_candidate_id, external_subagent_conflict_key, @@ -282,9 +283,10 @@ impl PromptCommandSourceProvider for FakeProvider { &self, command: &PromptCommandDefinition, arguments: &str, - ) -> Result { - Ok(ExpandedPromptCommand { + ) -> Result { + Ok(PromptCommandExpansion { content: command.template.replace("$ARGUMENTS", arguments), + workspace_file_references: vec!["src/lib.rs".to_string()], }) } @@ -308,6 +310,19 @@ fn capability_provider_contract_does_not_require_core_or_an_ecosystem_enum() { let snapshot = provider.discover(&context()).expect("discover fake source"); assert_eq!(snapshot.provider.ecosystem_id.as_str(), "fake.ecosystem"); assert_eq!(provider.watch_roots(&context()).len(), 1); + let expansion = provider + .expand(&snapshot.commands[0], "change") + .expect("prepare fake command expansion"); + assert_eq!(expansion.content, "fake-provider: change"); + assert_eq!(expansion.workspace_file_references, ["src/lib.rs"]); + + let final_result = ExpandedPromptCommand { + content: expansion.content, + }; + assert_eq!( + serde_json::to_value(final_result).unwrap(), + serde_json::json!({"content": "fake-provider: change"}) + ); } #[test] diff --git a/src/crates/services/services-core/src/bounded_fs.rs b/src/crates/services/services-core/src/bounded_fs.rs new file mode 100644 index 000000000..a0bcb7fc9 --- /dev/null +++ b/src/crates/services/services-core/src/bounded_fs.rs @@ -0,0 +1,170 @@ +//! Reusable bounded local filesystem reads and non-symlink traversal. + +use std::io::Read; +use std::path::{Path, PathBuf}; + +pub fn is_symlink_or_reparse(metadata: &std::fs::Metadata) -> bool { + if metadata.file_type().is_symlink() { + return true; + } + #[cfg(windows)] + { + use std::os::windows::fs::MetadataExt; + const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x0400; + return metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0; + } + #[cfg(not(windows))] + false +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum BoundedFileRead { + Content(Vec), + TooLarge, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum BoundedTextRead { + Content(String), + TooLarge, + InvalidUtf8, +} + +/// Reads at most `max_bytes + 1` bytes so concurrent growth cannot cause an +/// unbounded allocation. +pub fn read_bounded_file(path: &Path, max_bytes: usize) -> std::io::Result { + let file = std::fs::File::open(path)?; + let read_limit = max_bytes.saturating_add(1) as u64; + let mut bytes = Vec::with_capacity(max_bytes.min(64 * 1024).saturating_add(1)); + file.take(read_limit).read_to_end(&mut bytes)?; + if bytes.len() > max_bytes { + Ok(BoundedFileRead::TooLarge) + } else { + Ok(BoundedFileRead::Content(bytes)) + } +} + +pub fn read_bounded_text(path: &Path, max_bytes: usize) -> std::io::Result { + match read_bounded_file(path, max_bytes)? { + BoundedFileRead::Content(bytes) => Ok(match String::from_utf8(bytes) { + Ok(content) => BoundedTextRead::Content(content), + Err(_) => BoundedTextRead::InvalidUtf8, + }), + BoundedFileRead::TooLarge => Ok(BoundedTextRead::TooLarge), + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BoundedDirectoryWalkLimits { + pub max_depth: usize, + pub max_entries: usize, + pub max_directories: usize, + pub max_files: usize, +} + +impl BoundedDirectoryWalkLimits { + pub fn for_file_limit(max_files: usize) -> Self { + Self { + max_depth: 32, + max_entries: max_files.saturating_mul(4).max(1), + max_directories: max_files.max(1), + max_files, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BoundedDirectoryWalkLimit { + Depth, + Entries, + Directories, + Files, +} + +#[derive(Debug)] +pub enum BoundedDirectoryWalkError { + Io(std::io::Error), + LimitExceeded(BoundedDirectoryWalkLimit), +} + +impl std::fmt::Display for BoundedDirectoryWalkError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Io(error) => write!(formatter, "{error}"), + Self::LimitExceeded(limit) => write!(formatter, "{limit:?} limit exceeded"), + } + } +} + +impl std::error::Error for BoundedDirectoryWalkError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Io(error) => Some(error), + Self::LimitExceeded(_) => None, + } + } +} + +/// Iteratively collects matching regular files without following symlinks. +/// Limits apply to traversal cost, not only to matching files. +pub fn collect_bounded_regular_files( + root: &Path, + limits: BoundedDirectoryWalkLimits, + mut matches: impl FnMut(&Path) -> bool, +) -> Result, BoundedDirectoryWalkError> { + let metadata = match std::fs::symlink_metadata(root) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(BoundedDirectoryWalkError::Io(error)), + }; + if !metadata.is_dir() || is_symlink_or_reparse(&metadata) { + return Ok(Vec::new()); + } + + let mut files = Vec::new(); + let mut stack = vec![(root.to_path_buf(), 0usize)]; + let mut visited_entries = 0usize; + let mut visited_directories = 1usize; + while let Some((directory, depth)) = stack.pop() { + let entries = std::fs::read_dir(&directory).map_err(BoundedDirectoryWalkError::Io)?; + for entry in entries { + let entry = entry.map_err(BoundedDirectoryWalkError::Io)?; + visited_entries = visited_entries.saturating_add(1); + if visited_entries > limits.max_entries { + return Err(BoundedDirectoryWalkError::LimitExceeded( + BoundedDirectoryWalkLimit::Entries, + )); + } + let path = entry.path(); + let metadata = + std::fs::symlink_metadata(&path).map_err(BoundedDirectoryWalkError::Io)?; + if is_symlink_or_reparse(&metadata) { + continue; + } + if metadata.is_dir() { + let next_depth = depth.saturating_add(1); + if next_depth > limits.max_depth { + return Err(BoundedDirectoryWalkError::LimitExceeded( + BoundedDirectoryWalkLimit::Depth, + )); + } + visited_directories = visited_directories.saturating_add(1); + if visited_directories > limits.max_directories { + return Err(BoundedDirectoryWalkError::LimitExceeded( + BoundedDirectoryWalkLimit::Directories, + )); + } + stack.push((path, next_depth)); + } else if metadata.is_file() && matches(&path) { + if files.len() >= limits.max_files { + return Err(BoundedDirectoryWalkError::LimitExceeded( + BoundedDirectoryWalkLimit::Files, + )); + } + files.push(path); + } + } + } + files.sort(); + Ok(files) +} diff --git a/src/crates/services/services-core/src/lib.rs b/src/crates/services/services-core/src/lib.rs index 2f7abd224..6ae9961e0 100644 --- a/src/crates/services/services-core/src/lib.rs +++ b/src/crates/services/services-core/src/lib.rs @@ -3,6 +3,7 @@ //! This crate owns platform-agnostic service building blocks that can be //! tested without compiling the full BitFun product runtime. +pub mod bounded_fs; pub mod diagnostics; pub mod diff; #[cfg(feature = "dispatch-workspace")] @@ -33,3 +34,4 @@ pub mod token_usage; #[cfg(feature = "workspace-runtime")] pub mod workspace; pub mod workspace_instructions; +pub mod workspace_text; diff --git a/src/crates/services/services-core/src/markdown.rs b/src/crates/services/services-core/src/markdown.rs index cbb4e3a62..711e0ff73 100644 --- a/src/crates/services/services-core/src/markdown.rs +++ b/src/crates/services/services-core/src/markdown.rs @@ -11,6 +11,22 @@ static PROMPT_ARGUMENT_REGEX: LazyLock = LazyLock::new(|| { .expect("prompt argument regex pattern is valid") }); +/// Returns a conservative upper bound for prompt argument expansion without +/// allocating the expanded string. Each `$` may begin at most one placeholder; +/// the fallback arguments section is included even when a placeholder exists. +pub fn prompt_template_expansion_upper_bound(template: &str, arguments: &str) -> Option { + let placeholder_bound = template + .bytes() + .filter(|byte| *byte == b'$') + .count() + .checked_mul(arguments.len())?; + template + .len() + .checked_add(placeholder_bound)? + .checked_add(arguments.len())? + .checked_add("\n\nARGUMENTS: ".len()) +} + /// Expands Claude-compatible prompt arguments without executing dynamic content. pub fn expand_prompt_template_arguments(template: &str, arguments: &str) -> String { expand_prompt_template_arguments_with_names(template, arguments, &[]) diff --git a/src/crates/services/services-core/src/workspace_text.rs b/src/crates/services/services-core/src/workspace_text.rs new file mode 100644 index 000000000..e3a81792c --- /dev/null +++ b/src/crates/services/services-core/src/workspace_text.rs @@ -0,0 +1,272 @@ +//! Safe bounded reads for explicitly referenced local workspace text files. + +use crate::bounded_fs::{is_symlink_or_reparse, read_bounded_text, BoundedTextRead}; +use std::path::{Path, PathBuf}; +use thiserror::Error; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WorkspaceTextFile { + pub relative_path: String, + pub content: String, + pub byte_len: usize, +} + +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum WorkspaceTextReadError { + #[error("workspace root must be an absolute local directory")] + InvalidWorkspaceRoot, + #[error("file reference must be a safe workspace-relative path")] + InvalidRelativePath, + #[error("referenced workspace file was not found")] + NotFound, + #[error("referenced workspace path is not a regular file")] + NotRegularFile, + #[error("referenced workspace path must not contain symlinks")] + Symlink, + #[error("referenced file resolves outside the workspace")] + OutsideWorkspace, + #[error("referenced workspace file exceeds the {max_bytes} byte limit")] + TooLarge { max_bytes: usize }, + #[error("referenced workspace file is not valid UTF-8")] + InvalidUtf8, + #[error("failed to read referenced workspace file: {0}")] + Io(String), +} + +pub async fn read_workspace_relative_text_bounded( + workspace_root: &Path, + relative_path: &str, + max_bytes: usize, +) -> Result { + let normalized_path = normalize_workspace_relative_path(relative_path)?; + let components = normalized_path + .split('/') + .map(str::to_string) + .collect::>(); + let workspace_root = workspace_root.to_path_buf(); + let resolved = + tokio::task::spawn_blocking(move || resolve_workspace_file(&workspace_root, &components)) + .await + .map_err(|error| WorkspaceTextReadError::Io(error.to_string()))??; + + let bounded = tokio::task::spawn_blocking(move || read_bounded_text(&resolved, max_bytes)) + .await + .map_err(|error| WorkspaceTextReadError::Io(error.to_string()))? + .map_err(map_read_error)?; + let content = match bounded { + BoundedTextRead::Content(content) => content, + BoundedTextRead::TooLarge => return Err(WorkspaceTextReadError::TooLarge { max_bytes }), + BoundedTextRead::InvalidUtf8 => return Err(WorkspaceTextReadError::InvalidUtf8), + }; + let byte_len = content.len(); + Ok(WorkspaceTextFile { + relative_path: normalized_path, + content, + byte_len, + }) +} + +pub fn normalize_workspace_relative_path(value: &str) -> Result { + if value.is_empty() + || value.contains('\0') + || value.starts_with('~') + || value.starts_with('/') + || value.starts_with('\\') + || value.contains("://") + { + return Err(WorkspaceTextReadError::InvalidRelativePath); + } + + let normalized = value.replace('\\', "/"); + let mut components = Vec::new(); + for component in normalized.split('/') { + match component { + "" | "." => {} + ".." => return Err(WorkspaceTextReadError::InvalidRelativePath), + component if component.contains(':') => { + return Err(WorkspaceTextReadError::InvalidRelativePath) + } + component => components.push(component.to_string()), + } + } + if components.is_empty() { + return Err(WorkspaceTextReadError::InvalidRelativePath); + } + Ok(components.join("/")) +} + +fn resolve_workspace_file( + workspace_root: &Path, + components: &[String], +) -> Result { + if !workspace_root.is_absolute() { + return Err(WorkspaceTextReadError::InvalidWorkspaceRoot); + } + let canonical_root = std::fs::canonicalize(workspace_root).map_err(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + WorkspaceTextReadError::InvalidWorkspaceRoot + } else { + WorkspaceTextReadError::Io(error.to_string()) + } + })?; + if !std::fs::metadata(&canonical_root) + .map_err(|error| WorkspaceTextReadError::Io(error.to_string()))? + .is_dir() + { + return Err(WorkspaceTextReadError::InvalidWorkspaceRoot); + } + + let mut current = canonical_root.clone(); + for (index, component) in components.iter().enumerate() { + current.push(component); + let metadata = std::fs::symlink_metadata(¤t).map_err(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + WorkspaceTextReadError::NotFound + } else { + WorkspaceTextReadError::Io(error.to_string()) + } + })?; + if is_symlink_or_reparse(&metadata) { + return Err(WorkspaceTextReadError::Symlink); + } + let is_last = index + 1 == components.len(); + if is_last && !metadata.is_file() { + return Err(WorkspaceTextReadError::NotRegularFile); + } + if !is_last && !metadata.is_dir() { + return Err(WorkspaceTextReadError::NotFound); + } + } + + let canonical_file = std::fs::canonicalize(¤t).map_err(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + WorkspaceTextReadError::NotFound + } else { + WorkspaceTextReadError::Io(error.to_string()) + } + })?; + if !canonical_file.starts_with(&canonical_root) { + return Err(WorkspaceTextReadError::OutsideWorkspace); + } + Ok(canonical_file) +} + +fn map_read_error(error: std::io::Error) -> WorkspaceTextReadError { + if error.kind() == std::io::ErrorKind::NotFound { + WorkspaceTextReadError::NotFound + } else { + WorkspaceTextReadError::Io(error.to_string()) + } +} + +#[cfg(test)] +mod tests { + use super::{read_workspace_relative_text_bounded, WorkspaceTextReadError}; + use std::fs; + use std::path::Path; + + #[tokio::test] + async fn reads_nested_utf8_text_with_a_normalized_relative_path() { + let temp = tempfile::tempdir().unwrap(); + let file = temp.path().join("src/lib.rs"); + fs::create_dir_all(file.parent().unwrap()).unwrap(); + fs::write(&file, "pub fn answer() -> u8 { 42 }").unwrap(); + + let result = read_workspace_relative_text_bounded(temp.path(), "src/./lib.rs", 1024) + .await + .unwrap(); + + assert_eq!(result.relative_path, "src/lib.rs"); + assert_eq!(result.content, "pub fn answer() -> u8 { 42 }"); + assert_eq!(result.byte_len, 28); + } + + #[tokio::test] + async fn rejects_non_workspace_relative_syntax() { + let temp = tempfile::tempdir().unwrap(); + let invalid = [ + "", + ".", + "../secret.md", + "src/../../secret.md", + "/etc/passwd", + r"\server\share\secret.md", + r"C:\secret.md", + "~/secret.md", + "http://example.test/secret.md", + "file:///secret.md", + "src/bad\0name.md", + ]; + + for path in invalid { + let error = read_workspace_relative_text_bounded(temp.path(), path, 1024) + .await + .unwrap_err(); + assert_eq!( + error, + WorkspaceTextReadError::InvalidRelativePath, + "{path:?}" + ); + } + } + + #[tokio::test] + async fn distinguishes_missing_directories_oversize_and_invalid_utf8() { + let temp = tempfile::tempdir().unwrap(); + fs::create_dir_all(temp.path().join("docs")).unwrap(); + fs::write(temp.path().join("large.txt"), b"12345").unwrap(); + fs::write(temp.path().join("binary.txt"), [0xff, 0xfe]).unwrap(); + + assert_eq!( + read_workspace_relative_text_bounded(temp.path(), "missing.txt", 4) + .await + .unwrap_err(), + WorkspaceTextReadError::NotFound + ); + assert_eq!( + read_workspace_relative_text_bounded(temp.path(), "docs", 4) + .await + .unwrap_err(), + WorkspaceTextReadError::NotRegularFile + ); + assert_eq!( + read_workspace_relative_text_bounded(temp.path(), "large.txt", 4) + .await + .unwrap_err(), + WorkspaceTextReadError::TooLarge { max_bytes: 4 } + ); + assert_eq!( + read_workspace_relative_text_bounded(temp.path(), "binary.txt", 4) + .await + .unwrap_err(), + WorkspaceTextReadError::InvalidUtf8 + ); + } + + #[tokio::test] + async fn rejects_symlinked_references_even_when_the_target_is_inside() { + let temp = tempfile::tempdir().unwrap(); + fs::write(temp.path().join("target.txt"), "inside").unwrap(); + let link = temp.path().join("link.txt"); + if !create_file_symlink(Path::new("target.txt"), &link) { + return; + } + + assert_eq!( + read_workspace_relative_text_bounded(temp.path(), "link.txt", 1024) + .await + .unwrap_err(), + WorkspaceTextReadError::Symlink + ); + } + + #[cfg(unix)] + fn create_file_symlink(target: &Path, link: &Path) -> bool { + std::os::unix::fs::symlink(target, link).is_ok() + } + + #[cfg(windows)] + fn create_file_symlink(target: &Path, link: &Path) -> bool { + std::os::windows::fs::symlink_file(target, link).is_ok() + } +} diff --git a/src/crates/services/services-core/tests/markdown_owner_contracts.rs b/src/crates/services/services-core/tests/markdown_owner_contracts.rs index 963b93193..6ab838b67 100644 --- a/src/crates/services/services-core/tests/markdown_owner_contracts.rs +++ b/src/crates/services/services-core/tests/markdown_owner_contracts.rs @@ -1,6 +1,6 @@ use bitfun_services_core::markdown::{ expand_prompt_template_arguments, expand_prompt_template_arguments_with_names, - FrontMatterMarkdown, + prompt_template_expansion_upper_bound, FrontMatterMarkdown, }; use std::fs; @@ -36,6 +36,14 @@ fn prompt_arguments_expand_full_and_zero_based_quoted_positions() { ); } +#[test] +fn prompt_expansion_upper_bound_is_checked_without_rendering_the_result() { + let template = "$ARGUMENTS".repeat(1024); + let bound = prompt_template_expansion_upper_bound(&template, &"x".repeat(2048)).unwrap(); + + assert!(bound > 1024 * 1024); +} + #[test] fn prompt_arguments_preserve_missing_and_escaped_placeholders() { let expanded = expand_prompt_template_arguments(