diff --git a/docs/performance/01-compile-performance.md b/docs/performance/01-compile-performance.md index b5e036b18..3982e4e20 100644 --- a/docs/performance/01-compile-performance.md +++ b/docs/performance/01-compile-performance.md @@ -1,87 +1,101 @@ # BitFun 编译与依赖治理计划 -> 最近核实：2026-08-04 +> 最近核实：2026-08-10 > -> 快照基线：`gcwing/main@061024fb2` 加权限规划 owner 迁移 +> 实现复核基线：`gcwing/main@c6badb778` +> +> 性能 A/B 基线：`gcwing/main@1f538b96d` > > 稳定规则：[Rust 构建与依赖边界](../architecture/rust-build-dependency-boundaries.md) -这份文档只回答三个问题：当前主要成本在哪里、下一步先做什么、每轮治理如何证明有效。 -模块边界以架构文档为准，具体本地命令由最近的 `AGENTS.md` 维护，PR 只记录实际运行过的验证。 +这份文档只维护长期有用的信息：主要成本、已验证收益、下一步顺序和停止条件。模块边界以架构文档为准，具体本地命令由最近的 `AGENTS.md` 维护，单次 PR 的完整命令和日志留在 PR 中。 ## 1. 当前结论 | 结论 | 说明 | |---|---| -| 本轮收益是测试隔离，不是产品构建瘦身 | 权限纯策略测试从 Core 约 449 节点的闭包迁到 Agent Runtime 约 78 节点的闭包；产品依赖图不变 | -| 不再用 `product-full` 解决 focused test | Core 权限编排测试当前最小闭包是 `agent-runtime,canvas-runtime`；纯策略直接在 Agent Runtime 验证 | -| 不新增 CI 或测试入口 | 继续使用现有 test target 和 CI job；治理 PR 不复制同一闭包的验证 | -| 下一优先级是 App Server / Server | 先核实真实生产调用链，再收敛其 Core `product-full` 边界；收益不足则停止 | -| 依赖多版本不能按数量批量清理 | 只处理仓库能控制、行为等价且能缩小真实构建图的版本路径 | - -权限 owner 的长期边界和功能不变量见 -[Agent Runtime 服务设计](../architecture/agent-runtime-services-design.md)。这里不重复维护行为规格。 +| 本轮收敛的是测试链接拓扑 | Services 两个 crate 的集成 target 总数从 33 降到 25；选中的 `local-storage`、MCP、基础 SSH 闭包从 16 个集成 executable 降到 8 个 | +| 产品和第三方依赖闭包不变 | 没有修改 dependency、feature 或运行时代码，不能据此宣称产品构建已经变快 | +| 冷编译和 owner 重建没有显著提速 | Windows 单轮 A/B 不足以区分真实差异与运行波动；可确认的收益是少 8 个链接目标以及更少 EXE/PDB 产物 | +| focused test 仍保持精确 | 同 owner、feature、平台和进程语义的源文件进入分组 target；使用 `--test ::` 运行单模块 | +| CI 不再叠加同闭包验证 | 本轮只扩展已有边界检查器，不增加 workflow、job 或测试步骤 | +| App Server 暂缓 | 当前改造稳定前不继续据旧分析调整其 feature 或 owner；后续必须基于届时最新生产调用链重新评估 | -## 2. 治理原则 +## 2. 治理门槛 -目标是缩短常用开发、focused test、CI 和打包路径，同时保持产品行为与分层边界稳定。 -每个治理 PR 必须同时满足以下门槛： +目标是缩短常用开发、focused test、CI 和打包路径，同时保持产品行为与分层边界稳定。每个治理 PR 必须同时回答： | 门槛 | 必须回答的问题 | |---|---| -| Owner | 逻辑属于哪个现有 owner？是否存在真实生产消费者？ | -| 行为 | 本地、远程和平台差异如何保持？哪些等价测试保护它？ | -| 构建图 | 哪个产品或测试闭包实际退出了哪些依赖？ | -| 耗时 | 若宣称性能收益，是否在同机器、同命令、同缓存状态下测量？ | +| Owner | 逻辑属于哪个现有 owner？是否有真实生产消费者？ | +| 行为 | 本地、远程、平台、进程和失败语义如何保持？ | +| 构建图 | 哪个真实产品或测试闭包退出了哪些依赖或 target？ | +| 耗时 | 若宣称提速，是否在同机器、同命令和同缓存状态下测量？ | | 增量成本 | 是否新增 dependency、feature、test target、CI job 或长期兼容层？ | 以下做法不属于优化： - 用 `product-full`、`all-features` 或 workspace 全量测试掩盖 feature 边界； - 为减少重复版本数字强制 patch 平台依赖、宏生态或第三方兼容窗口； -- 新建第二套 Agent、Tool、Permission Runtime 或无消费者抽象； -- 未测量就引入 sccache、替换链接器、合并 Installer workspace 或增加 CI job； -- 删除跨平台行为保护来换取表面 CI 时长。 +- 为统一形式新建第二套 Runtime、状态 owner、传输层或无消费者抽象； +- 未测量就引入 sccache、替换链接器、合并独立 workspace 或增加 CI job； +- 删除跨平台、负向能力或异常进程行为保护来换取表面时长。 ## 3. 当前基线 -### 3.1 Rust 构建图 +### 3.1 服务层测试拓扑 + +本轮只合并 owner 和运行边界相同的测试。`session_write_lock_contracts` 依赖当前测试 executable 启动异常退出子进程，因此继续保持独立；不同 feature 的服务测试也不合并。 + +| 范围 | 变更前 target | 变更后 target | 集成测试数 | +|---|---:|---:|---:| +| `services-core` 全部 | 20 | 13 | 不变 | +| `services-core/local-storage` | 12 | 5 | 58 | +| `services-integrations` 全部 | 13 | 12 | 不变 | +| MCP | 2 | 2 | 45 | +| 基础 Remote SSH | 2 | 1 | 11 | + +Windows、Cargo 1.97.1 的同机独立 `CARGO_TARGET_DIR` A/B 如下。冷构建、无变更重跑和 +单叶文件 mtime 触发各测一次；“owner 重建”在依赖已热后对 owner package 执行三轮 +clean/rebuild，表中为均值。时间是方向性证据，不是硬阈值。 + +| 闭包 | 冷构建前→后 | 无变更前→后 | 单叶变更前→后 | owner 重建前→后 | +|---|---:|---:|---:|---:| +| local-storage | 22.14s → 22.04s | 0.56s → 0.55s | 0.99s → 1.06s | 8.30s → 8.16s | +| 基础 Remote SSH | 27.60s → 28.35s | 0.61s → 0.62s | 1.22s → 1.30s | 3.49s → 3.49s | + +这些单轮数据不支持“编译明显提速”的结论，也不足以把小幅差值与机器波动区分开。依赖编译仍占 +冷路径主导；分组后单叶变更会重链整个职责 target，模块过滤只减少实际运行的测试，不减少该 target +的编译和链接。分组还会降低测试进程级故障隔离粒度，因此当前只合并相同失败域，没有继续扩大。 -| 路径 | 当前快照 | 判断 | -|---|---:|---| -| `bitfun-core` | 约 493 个 Rust 文件、243,900 行 | 仍是最大的高频失效面；只按真实 owner 做纵向迁移 | -| Core 直接消费者 | ACP、App Server、CLI、Desktop、SDK Host、Server | 每次只迁移一个有真实调用方的服务切片 | -| Agent Runtime focused test | 约 78 个唯一 package/version 节点 | 适合无 IO 的 Agent Runtime 纯决策测试 | -| Core `agent-runtime` check | 约 391 个节点 | 窄 owner feature 可独立编译 | -| Core 权限编排测试 | `agent-runtime,canvas-runtime`，约 449 个节点 | 保留真实 scope、Hook、请求生命周期和 Tool 执行 | -| Core `product-full` test | 约 516 个节点 | 仅用于确实需要完整产品装配的兼容路径 | -| Agent Runtime integration target | 5 个显式 target | 已完成收敛；平台和进程边界继续独立 | +MCP 的 2→1 candidate 也做过同口径 A/B，但冷构建和 owner 重建均无可区分的提速；streamable HTTP +测试还拥有真实 loopback TCP/SSE/超时失败域，因此最终继续保持两个 target，不计入本轮收益。 -节点数来自同一 Windows 环境下的 `cargo tree --locked` 相对统计，不是实际耗时，也不是跨平台阈值。 -权限纯策略路径理论上少进入约 371 个节点；产品构建闭包没有变化。 +可重复确认的产物变化如下；`test executable` 包含每个 crate 的 lib test harness，因此比 integration +target 多 1。PDB 大小会随工具链变化，只比较同次 A/B： + +| 闭包 | test executable | EXE | PDB | +|---|---:|---:|---:| +| local-storage | 13 → 6 | 25.2 → 19.2 MiB | 135.7 → 91.9 MiB | +| 基础 Remote SSH | 3 → 2 | 3.9 → 2.8 MiB | 53.5 → 43.8 MiB | ### 3.2 依赖与 feature | 状态 | 范围 | 处理结论 | |---|---|---| -| 已稳定 | 根 `Cargo.lock`、Reqwest Rustls 单栈、Desktop 直接 `image 0.25`、workspace Tokio 最小基线 | 不重复治理 | -| 下一步核实 | App Server / Server 的 Core `product-full` | 按生产 construction path 收敛，不先写 feature 清单 | -| 可独立治理 | Installer 的 Reqwest 0.12、独立 lockfile、疑似无消费者的 `tokio/full` | 保持 Installer 独立 workspace，不顺手合并 | -| 等待上游 | `screenshots 0.8.10 -> image 0.24.9` | 只有受维护且行为等价的上游替代出现后再处理 | +| 已稳定 | 根 `Cargo.lock`、Reqwest Rustls 单栈、workspace Tokio 最小基线 | 不重复治理 | +| 当前不动 | App Server / Server | 等正在进行的 App Server 改造稳定后，按最新 production construction path 重新测量 | +| 下一轮核实 | Installer 独立 workspace 的 `tokio/full`、Reqwest 和下载/解压闭包 | 保持 workspace 与产品生命周期独立；只删除真实未使用能力，不为版本统一强行并入根 workspace | +| 等待上游 | `screenshots -> image 0.24` | 只有受维护且行为等价的上游替代出现后再处理 | | 明确保留 | `portable-pty 0.8/0.9` | 非 OHOS 与 OHOS 的平台兼容选择，不为去重破坏 | -根 lockfile 约有 116 个名称存在多版本。这个数字只用于发现候选，不能直接转化为治理任务。 -`oxc`、`rquickjs`、vendored `git2`、`sherpa-onnx` 等重依赖都有真实 capability owner；只有某个产品入口 -不消费对应能力时，才允许让它退出该入口的构建图。 +重复版本数量只用于发现候选，不能直接转化为治理任务。`oxc`、`rquickjs`、vendored `git2`、`sherpa-onnx` 等重依赖都有真实 capability owner；只有某个产品入口不消费对应能力时，才允许让它退出该入口的构建图。 ### 3.3 CI 与本地验证 -- 现有 CI 已覆盖 workspace check、Core/Desktop lib、平台敏感 owner 测试和独立 runtime/CLI 验证； - 不再为治理 PR 叠加同闭包 job。 -- 本地先运行 owner 文档维护的最小 package/target/feature 命令。广泛 build、workspace suite、打包和 - 平台矩阵由 CI 承担，除非改动直接影响这些路径或需要复现 CI 故障。 -- CI 收敛必须基于多次 job/step 耗时、缓存状态、平台事实和失败历史。测试名称相似不等于覆盖重复， - `SKIPPED`、未触发或只编译未运行也不等于通过。 +- CI 承担 workspace、真实产品 feature、跨平台、完整测试和打包；不为治理 PR 复制同一依赖闭包。 +- 本地从 owner 文档的最小 package/target/feature 入口开始；仅名称过滤不能阻止无关 target 编译。 +- CI 收敛必须基于多次 job/step 耗时、缓存状态、平台事实和失败历史。`SKIPPED`、未触发或只编译未运行都不算通过。 ## 4. 已完成，不再重复实施 @@ -94,53 +108,47 @@ | CI 拓扑 | Rust job 不再等待完整前端构建，自建 Tauri 检查所需资源目录 | | 依赖收敛 | Desktop 直接 image 版本和 Reqwest TLS 双栈已治理 | | Agent Runtime 测试 | 28 个 integration executable 已收敛为 5 个职责/平台 target | +| Services 测试 | 两个服务 crate 使用显式 target；选中闭包少 8 个 integration executable，进程/feature/external-system 边界保持独立 | -内置 Agent 内容已经移到无第三方依赖的 `bitfun-agent-content`，减少了 Core build-script 工作；但 Core -仍直接依赖该 crate。没有足够产品收益前，不为消除这一编译指纹引入动态 provider、运行时文件读取或资源协议。 +内置 Agent 内容已经移到无第三方依赖的 `bitfun-agent-content`，减少了 Core build-script 工作； +但 Core 仍直接依赖该 crate。没有足够产品收益前，不为消除这一编译指纹引入动态 provider、 +运行时文件读取或资源协议。 ## 5. 后续顺序 -### R1：收敛 App Server / Server 的 `product-full` 边界 - -这是下一 PR 的推荐范围，也是唯一需要优先设计的核心入口改造。 - -先回答： +### R1：Installer 独立构建闭包核实 -- App Server 与 Server 的真实 construction、command 和 schema 路径分别消费哪些 Core owner？ -- Server 对未实现能力应保持什么 typed unsupported 行为？ -- 哪些能力由 Server 直接消费，哪些只是经 App Server 间接带入？ +下一轮先审查 Installer 的实际异步 API、下载流、事件流、解压、代理、证书和三平台打包路径， +再决定是否缩小 Tokio feature、更新 Reqwest 或删除重复依赖。Installer 继续保持独立 workspace +和发布生命周期；根 workspace 的版本不是强制目标。 -实现边界： +启动实现必须同时满足： -- 只替换一个端到端 capability slice 的依赖路径，不一次迁移全部 Core 调用； -- 优先显式选择已有 owner feature，或消费现有 Runtime SDK/service port； -- 不复制 Session、Tool、Permission、Hook、Event 状态，不建立第二 Runtime； -- 未迁移能力保留现有兼容路径或明确 unsupported，禁止静默本机回退。 +- 从真实 API 使用反推出 Tokio/Reqwest 最小能力，而不是猜测 feature 清单； +- 下载进度、取消、断点/失败清理、代理和证书错误行为有等价测试； +- Windows、macOS、Linux 的安装/快捷方式差异不因依赖收敛被抹平； +- 冷构建、增量构建、二进制大小或重复依赖至少一项有可复现实测收益；收益不足时只提交看护或停止，不做版本迁移。 -验收必须覆盖 Server WebSocket/App Server round-trip、权限、取消、事件与恢复语义，并对比 App Server、 -Server 的 normal/build/test closure。若构建图收益不足或行为等价无法证明，则不删除兼容边界。 +### 暂缓与候选 -### 后续队列 - -| 顺序 | 范围 | 启动条件 | -|---|---|---| -| R2 | 从 ACP 迁移一个已有 Services owner 的 host-service 切片 | 明确真实调用方，并能保持 Windows 进程树、SSH、取消和远程身份语义 | -| R3 | Installer lockfile、Reqwest 0.13 与无消费者依赖治理 | 下载、SSE/进度、取消、代理、证书失败和三平台 packaging 可验证 | -| R4 | 消除 `screenshots -> image 0.24` | 有受维护、无需 fork/vendoring 且屏幕枚举/DPI/权限行为等价的上游路径 | +| 范围 | 启动条件 | +|---|---| +| App Server / Server | 当前改造合入并稳定后，重新锁定最新生产调用链和可信 owner 边界 | +| 产品入口重型 capability | 证明某个交付形态不消费该能力，并具备 typed unsupported/fallback 行为 | +| `screenshots -> image 0.24` | 有受维护、无需 fork/vendoring 且屏幕枚举、DPI、权限行为等价的上游路径 | 每一步都在前一 PR 合入后的最新 main 重新测量。无法证明边界或收益时停止，不为了完成清单继续重构。 ## 6. 每轮 PR 的证据 -PR 描述只需维护一张简表，不新增全仓依赖台账： +PR 描述维护一张简表即可，不新增全仓依赖台账： | 证据 | 变更前 | 变更后 | |---|---:|---:| | 真实产品 normal/build closure | | | -| owner focused-test closure | | | -| 目标重复版本或重型依赖路径 | | | +| owner focused-test closure/target | | | | 冷、热或增量耗时（同机器、命令、缓存状态） | | | +| 产物数量/大小 | | | | 新增 dependency、feature、test target、CI job | | | -同时记录功能不变量、远程/平台差异、实际运行的最小验证和未运行的 CI。若产品 closure 不变，只能说明 -focused-test 或 owner 边界收益，不能宣称产品构建已经变快。 +同时记录功能不变量、远程/平台差异、实际运行的最小验证和未运行的 CI。若产品 closure 不变，只能说明测试拓扑或 owner 边界收益，不能宣称产品构建已经变快。 diff --git a/scripts/check-core-boundaries.test.mjs b/scripts/check-core-boundaries.test.mjs index 62c59a768..605bb1ea8 100644 --- a/scripts/check-core-boundaries.test.mjs +++ b/scripts/check-core-boundaries.test.mjs @@ -20,7 +20,11 @@ import { } from './core-boundaries/cargo-dependency-boundaries.mjs'; import { checkCliIntegrationTestTopology, + checkServicesCoreIntegrationTestTopology, + checkServicesIntegrationsIntegrationTestTopology, cliIntegrationTestTargets, + servicesCoreIntegrationTestTargets, + servicesIntegrationsIntegrationTestTargets, validateExplicitIntegrationTestTopology, } from './core-boundaries/explicit-test-topology.mjs'; import { crateLayoutRules } from './core-boundaries/rules/crate-layout.mjs'; @@ -120,6 +124,21 @@ test('feature-gated integration targets require every positive crate feature', ( assert.doesNotMatch(violations[0].message, /remote-ssh-concrete.*missing/); }); +test('feature-gated integration target parsing skips a Rust shebang', () => { + const sourcePath = join(TEST_ROOT, 'tests', 'remote.rs'); + const pkg = { + ...packageAt('example', 'src/crates/services/example/Cargo.toml'), + targets: [integrationTarget('remote', sourcePath, ['remote-ssh'])], + }; + + const violations = findFeatureGatedTestTargetViolations([pkg], { + readSource: () => '#!/usr/bin/env rustx\n#![cfg(all(feature = "remote-ssh", feature = "workspace-search"))]\n', + }); + + assert.equal(violations.length, 1); + assert.match(violations[0].message, /workspace-search/); +}); + test('matching integration target requirements cover all positive crate features', () => { const sourcePath = join(TEST_ROOT, 'tests', 'remote.rs'); const pkg = { @@ -242,6 +261,42 @@ test('CLI integration tests keep the reviewed three-target topology', () => { assert.deepEqual(checkCliIntegrationTestTopology(repositoryRoot), []); }); +test('service integration tests keep their reviewed explicit target topology', () => { + const repositoryRoot = fileURLToPath(new URL('..', import.meta.url)); + + assert.deepEqual(servicesCoreIntegrationTestTargets, [ + { name: 'markdown_owner_contracts', path: 'tests/markdown_owner_contracts.rs' }, + { name: 'declarative_workspace_instruction_contracts', path: 'tests/declarative_workspace_instruction_contracts.rs' }, + { name: 'lsp_plugin_registry_contracts', path: 'tests/lsp_plugin_registry_contracts.rs' }, + { name: 'runtime_ownership_contracts', path: 'tests/runtime_ownership_contracts.rs' }, + { name: 'local_runtime_ports', path: 'tests/local_runtime_ports.rs' }, + { name: 'permission_store_contracts', path: 'tests/permission_store_contracts.rs' }, + { name: 'workspace_instruction_contracts', path: 'tests/workspace_instruction_contracts.rs' }, + { name: 'session_write_lock_contracts', path: 'tests/session_write_lock_contracts.rs' }, + { name: 'process_runtime_contracts', path: 'tests/process_runtime_contracts.rs' }, + { name: 'service_contracts', path: 'tests/service_contracts.rs' }, + { name: 'storage_owner_contracts', path: 'tests/storage_owner_contracts.rs' }, + { name: 'session_contracts', path: 'tests/session_contracts.rs' }, + { name: 'session_usage_contracts', path: 'tests/session_usage_contracts.rs' }, + ]); + assert.deepEqual(servicesIntegrationsIntegrationTestTargets, [ + { name: 'debug_log_owner_contracts', path: 'tests/debug_log_owner_contracts.rs' }, + { name: 'script_tool_runtime', path: 'tests/script_tool_runtime.rs' }, + { name: 'announcement_contracts', path: 'tests/announcement_contracts.rs' }, + { name: 'file_watch_contracts', path: 'tests/file_watch_contracts.rs' }, + { name: 'function_agent_contracts', path: 'tests/function_agent_contracts.rs' }, + { name: 'git_contracts', path: 'tests/git_contracts.rs' }, + { name: 'mcp_contracts', path: 'tests/mcp_contracts.rs' }, + { name: 'mcp_streamable_http_contracts', path: 'tests/mcp_streamable_http_contracts.rs' }, + { name: 'remote_connect_contracts', path: 'tests/remote_connect_contracts.rs' }, + { name: 'remote_ssh_contracts', path: 'tests/remote_ssh_contracts.rs' }, + { name: 'remote_workspace_search_disabled_contracts', path: 'tests/remote_workspace_search_disabled_contracts.rs' }, + { name: 'workspace_search_contracts', path: 'tests/workspace_search_contracts.rs' }, + ]); + assert.deepEqual(checkServicesCoreIntegrationTestTopology(repositoryRoot), []); + assert.deepEqual(checkServicesIntegrationsIntegrationTestTopology(repositoryRoot), []); +}); + test('runtime-services test support is absent from ordinary library builds', async () => { const [manifest, library] = await Promise.all([ readFile( diff --git a/scripts/core-boundaries/cargo-dependency-boundaries.mjs b/scripts/core-boundaries/cargo-dependency-boundaries.mjs index 0c00d7ca8..e07c97e16 100644 --- a/scripts/core-boundaries/cargo-dependency-boundaries.mjs +++ b/scripts/core-boundaries/cargo-dependency-boundaries.mjs @@ -1045,6 +1045,44 @@ export function findProductEntrypointCoreFeatureViolations( return violations; } +function skipRustComment(source, startIndex) { + if (source.startsWith('//', startIndex)) { + const lineEnd = source.indexOf('\n', startIndex + 2); + return lineEnd === -1 ? source.length : lineEnd + 1; + } + if (!source.startsWith('/*', startIndex)) { + return startIndex; + } + + let depth = 1; + let index = startIndex + 2; + while (index < source.length && depth > 0) { + if (source.startsWith('/*', index)) { + depth += 1; + index += 2; + } else if (source.startsWith('*/', index)) { + depth -= 1; + index += 2; + } else { + index += 1; + } + } + return index; +} + +function skipRustRawString(source, startIndex) { + if (!['b', 'c', 'r'].includes(source[startIndex])) { + return startIndex; + } + const opening = /^(?:b|c)?r(#{0,255})"/.exec(source.slice(startIndex)); + if (opening === null) { + return startIndex; + } + const closing = `"${opening[1]}`; + const closingIndex = source.indexOf(closing, startIndex + opening[0].length); + return closingIndex === -1 ? source.length : closingIndex + closing.length; +} + function matchingClosingDelimiter( source, openingIndex, @@ -1067,6 +1105,16 @@ function matchingClosingDelimiter( } continue; } + const rawStringEnd = skipRustRawString(source, index); + if (rawStringEnd !== index) { + index = rawStringEnd - 1; + continue; + } + const commentEnd = skipRustComment(source, index); + if (commentEnd !== index) { + index = commentEnd - 1; + continue; + } if (character === '"' || character === "'") { quote = character; } else if (character === openingCharacter) { @@ -1086,57 +1134,79 @@ function matchingClosingParenthesis(source, openingIndex) { return matchingClosingDelimiter(source, openingIndex, '(', ')'); } -function crateCfgBodies(source) { - const bodies = []; - let index = source.charCodeAt(0) === 0xFEFF ? 1 : 0; - +function skipRustTrivia(source, startIndex) { + let index = startIndex; while (index < source.length) { if (/\s/.test(source[index])) { index += 1; continue; } - if (source.startsWith('//', index)) { + const commentEnd = skipRustComment(source, index); + if (commentEnd !== index) { + index = commentEnd; + continue; + } + break; + } + return index; +} + +function leadingCrateAttributes(source) { + const attributes = []; + let index = source.charCodeAt(0) === 0xFEFF ? 1 : 0; + if (source.startsWith('#!', index)) { + const afterBang = skipRustTrivia(source, index + 2); + if (source[afterBang] !== '[') { const lineEnd = source.indexOf('\n', index + 2); index = lineEnd === -1 ? source.length : lineEnd + 1; - continue; } - if (source.startsWith('/*', index)) { - let depth = 1; - index += 2; - while (index < source.length && depth > 0) { - if (source.startsWith('/*', index)) { - depth += 1; - index += 2; - } else if (source.startsWith('*/', index)) { - depth -= 1; - index += 2; - } else { - index += 1; - } - } - continue; + } + + while (index < source.length) { + index = skipRustTrivia(source, index); + if (source[index] !== '#') { + break; } - if (!source.startsWith('#![', index)) { + let cursor = skipRustTrivia(source, index + 1); + if (source[cursor] !== '!') { + break; + } + cursor = skipRustTrivia(source, cursor + 1); + if (source[cursor] !== '[') { break; } - const closingBracket = matchingClosingDelimiter(source, index + 2, '[', ']'); + const closingBracket = matchingClosingDelimiter(source, cursor, '[', ']'); if (closingBracket === -1) { break; } - const attribute = source.slice(index, closingBracket + 1); - const cfgStart = /^#!\s*\[\s*cfg\s*\(/.exec(attribute); - if (cfgStart !== null) { - const openingIndex = cfgStart[0].length - 1; - const closingIndex = matchingClosingParenthesis(attribute, openingIndex); - if (closingIndex !== -1) { - bodies.push(attribute.slice(openingIndex + 1, closingIndex)); + const nameStart = skipRustTrivia(source, cursor + 1); + const nameMatch = /^(?:r#)?([A-Za-z_][A-Za-z0-9_]*)/.exec(source.slice(nameStart)); + const name = nameMatch?.[1] ?? ''; + const argumentStart = skipRustTrivia(source, nameStart + (nameMatch?.[0].length ?? 0)); + let body = null; + if (source[argumentStart] === '(') { + const closingParenthesis = matchingClosingParenthesis(source, argumentStart); + if (closingParenthesis !== -1 && closingParenthesis < closingBracket) { + body = source.slice(argumentStart + 1, closingParenthesis); } } + attributes.push({ name, body }); index = closingBracket + 1; } - return bodies; + return attributes; +} + +export function crateCfgBodies(source) { + return leadingCrateAttributes(source) + .filter(({ name, body }) => name === 'cfg' && body !== null) + .map(({ body }) => body); +} + +export function hasCrateCfgAttr(source) { + return leadingCrateAttributes(source) + .some(({ name }) => name === 'cfg_attr'); } function removeCfgBranches(expression, branchName) { diff --git a/scripts/core-boundaries/checker.mjs b/scripts/core-boundaries/checker.mjs index fbd68a371..0742b296e 100644 --- a/scripts/core-boundaries/checker.mjs +++ b/scripts/core-boundaries/checker.mjs @@ -40,6 +40,7 @@ import { agentRuntimeIntegrationTestTargets, checkAgentRuntimeIntegrationTestTopology, checkCliIntegrationTestTopology, + checkServiceIntegrationTestTopologies, cliIntegrationTestTargets, validateExplicitIntegrationTestTopology, } from './explicit-test-topology.mjs'; @@ -1122,6 +1123,7 @@ export function runCoreBoundaryCheck() { failures.push(...checkCargoDependencyBoundariesSafely({ root: ROOT, crateLayoutRules })); failures.push(...checkAgentRuntimeIntegrationTestTopology(ROOT)); failures.push(...checkCliIntegrationTestTopology(ROOT)); + failures.push(...checkServiceIntegrationTestTopologies(ROOT)); for (const rule of forbiddenManifestDependencyRules) { checkForbiddenManifestDependencyRule(rule); diff --git a/scripts/core-boundaries/explicit-test-topology.mjs b/scripts/core-boundaries/explicit-test-topology.mjs index e43a0659c..f0addde7a 100644 --- a/scripts/core-boundaries/explicit-test-topology.mjs +++ b/scripts/core-boundaries/explicit-test-topology.mjs @@ -1,6 +1,8 @@ import { readdirSync, readFileSync } from 'node:fs'; import { join, posix, relative } from 'node:path'; +import { crateCfgBodies, hasCrateCfgAttr } from './cargo-dependency-boundaries.mjs'; + export const agentRuntimeIntegrationTestTargets = [ { name: 'agent_definition_contracts', path: 'tests/agent_definition_contracts.rs' }, { name: 'agent_interaction_contracts', path: 'tests/agent_interaction_contracts.rs' }, @@ -15,6 +17,37 @@ export const cliIntegrationTestTargets = [ { name: 'terminal_process_contracts', path: 'tests/terminal_process_contracts.rs' }, ]; +export const servicesCoreIntegrationTestTargets = [ + { name: 'markdown_owner_contracts', path: 'tests/markdown_owner_contracts.rs' }, + { name: 'declarative_workspace_instruction_contracts', path: 'tests/declarative_workspace_instruction_contracts.rs' }, + { name: 'lsp_plugin_registry_contracts', path: 'tests/lsp_plugin_registry_contracts.rs' }, + { name: 'runtime_ownership_contracts', path: 'tests/runtime_ownership_contracts.rs' }, + { name: 'local_runtime_ports', path: 'tests/local_runtime_ports.rs' }, + { name: 'permission_store_contracts', path: 'tests/permission_store_contracts.rs' }, + { name: 'workspace_instruction_contracts', path: 'tests/workspace_instruction_contracts.rs' }, + { name: 'session_write_lock_contracts', path: 'tests/session_write_lock_contracts.rs' }, + { name: 'process_runtime_contracts', path: 'tests/process_runtime_contracts.rs' }, + { name: 'service_contracts', path: 'tests/service_contracts.rs' }, + { name: 'storage_owner_contracts', path: 'tests/storage_owner_contracts.rs' }, + { name: 'session_contracts', path: 'tests/session_contracts.rs' }, + { name: 'session_usage_contracts', path: 'tests/session_usage_contracts.rs' }, +]; + +export const servicesIntegrationsIntegrationTestTargets = [ + { name: 'debug_log_owner_contracts', path: 'tests/debug_log_owner_contracts.rs' }, + { name: 'script_tool_runtime', path: 'tests/script_tool_runtime.rs' }, + { name: 'announcement_contracts', path: 'tests/announcement_contracts.rs' }, + { name: 'file_watch_contracts', path: 'tests/file_watch_contracts.rs' }, + { name: 'function_agent_contracts', path: 'tests/function_agent_contracts.rs' }, + { name: 'git_contracts', path: 'tests/git_contracts.rs' }, + { name: 'mcp_contracts', path: 'tests/mcp_contracts.rs' }, + { name: 'mcp_streamable_http_contracts', path: 'tests/mcp_streamable_http_contracts.rs' }, + { name: 'remote_connect_contracts', path: 'tests/remote_connect_contracts.rs' }, + { name: 'remote_ssh_contracts', path: 'tests/remote_ssh_contracts.rs' }, + { name: 'remote_workspace_search_disabled_contracts', path: 'tests/remote_workspace_search_disabled_contracts.rs' }, + { name: 'workspace_search_contracts', path: 'tests/workspace_search_contracts.rs' }, +]; + function parseExplicitTestTargets(manifestText) { const targets = []; let current = null; @@ -66,7 +99,11 @@ function parseFlatRootModules(root, source, errors) { let valid = true; for (let index = 0; index < lines.length; index += 1) { const line = lines[index].trim(); - if (line === '' || line.startsWith('//!')) { + if ( + line === '' + || line.startsWith('//!') + || /^#!\[cfg\(feature = "[A-Za-z0-9_-]+"\)\]$/.test(line) + ) { continue; } const pathAttribute = line.match(/^#\[path\s*=\s*"([^"]+)"\]$/); @@ -82,12 +119,79 @@ function parseFlatRootModules(root, source, errors) { return valid ? references : []; } +function positiveFeatureGates(expression) { + const withoutNegativeFeatures = expression.replace( + /\bnot\s*\(\s*feature\s*=\s*"[^"]+"\s*\)/g, + '', + ); + return Array.from( + withoutNegativeFeatures.matchAll(/\bfeature\s*=\s*"([^"]+)"/g), + (match) => match[1], + ); +} + +function negativeFeatureGates(expression) { + return Array.from( + expression.matchAll(/\bnot\s*\(\s*feature\s*=\s*"([^"]+)"\s*\)/g), + (match) => match[1], + ); +} + +function validateGroupedLeafCfg( + root, + rootSource, + leaf, + leafSource, + allowedLeafNegativeFeatures, + errors, +) { + const rootFeatures = new Set( + crateCfgBodies(rootSource).flatMap(positiveFeatureGates), + ); + const allowedNegativeFeatures = allowedLeafNegativeFeatures.get(leaf) ?? new Set(); + if (hasCrateCfgAttr(leafSource)) { + errors.push(`grouped test leaf ${leaf} uses unsupported crate-level cfg_attr`); + } + for (const expression of crateCfgBodies(leafSource)) { + const unsupportedCfg = expression + .replace(/\bfeature\s*=\s*"[^"]+"/g, '') + .replace(/\b(?:all|not)\b/g, '') + .replace(/[\s(),]/g, ''); + const hasComplexNegativeGate = /\bnot\s*\(\s*(?!feature\s*=)/.test(expression); + if (/\bany\s*\(/.test(expression) || hasComplexNegativeGate || unsupportedCfg !== '') { + errors.push( + `grouped test leaf ${leaf} has a platform or unsupported crate cfg outside root ${root}`, + ); + } + for (const feature of positiveFeatureGates(expression)) { + if (!rootFeatures.has(feature)) { + errors.push( + `grouped test leaf ${leaf} adds positive feature ${feature} outside root ${root}`, + ); + } + } + for (const feature of negativeFeatureGates(expression)) { + if (rootFeatures.has(feature)) { + errors.push( + `grouped test leaf ${leaf} negates root feature ${feature} from ${root}`, + ); + } else if (!allowedNegativeFeatures.has(feature)) { + errors.push( + `grouped test leaf ${leaf} adds unreviewed negative feature ${feature} outside root ${root}`, + ); + } + } + } +} + export function validateExplicitIntegrationTestTopology({ manifestText, expectedTargets, topLevelRustFiles, rootSources, leafRustFiles, + leafSources, + allowedLeafNegativeFeatures = new Map(), }) { const errors = []; if (!packageDisablesAutotests(manifestText)) { @@ -130,6 +234,19 @@ export function validateExplicitIntegrationTestTopology({ errors.push(`test root ${root} references missing leaf: ${leaf}`); continue; } + const leafSource = leafSources.get(leaf); + if (leafSource === undefined) { + errors.push(`missing grouped test leaf source: ${leaf}`); + continue; + } + validateGroupedLeafCfg( + root, + source, + leaf, + leafSource, + allowedLeafNegativeFeatures, + errors, + ); const expectedModuleName = posix.basename(leaf, '.rs'); if (reference.moduleName !== expectedModuleName) { errors.push(`test leaf ${leaf} must use module name ${expectedModuleName}`); @@ -147,17 +264,18 @@ export function validateExplicitIntegrationTestTopology({ return errors; } -function collectRustFiles(dir, testsDir, files, ignoredDirectories) { +function collectRustFiles(dir, testsDir, files, sources, ignoredDirectories) { for (const entry of readdirSync(dir, { withFileTypes: true })) { const path = join(dir, entry.name); if (entry.isDirectory()) { const repoPath = `tests/${relative(testsDir, path).replaceAll('\\', '/')}`; if (!ignoredDirectories.has(repoPath)) { - collectRustFiles(path, testsDir, files, ignoredDirectories); + collectRustFiles(path, testsDir, files, sources, ignoredDirectories); } } else if (entry.isFile() && entry.name.endsWith('.rs')) { const repoPath = `tests/${relative(testsDir, path).replaceAll('\\', '/')}`; files.push(repoPath); + sources.set(repoPath, readFileSync(path, 'utf8')); } } } @@ -166,6 +284,7 @@ function checkExplicitIntegrationTestTopology(root, { cratePath, expectedTargets, ignoredDirectories = [], + allowedLeafNegativeFeatures = new Map(), }) { const crateDir = join(root, ...cratePath.split('/')); const testsDir = join(crateDir, 'tests'); @@ -173,6 +292,7 @@ function checkExplicitIntegrationTestTopology(root, { const topLevelRustFiles = []; const leafRustFiles = []; const rootSources = new Map(); + const leafSources = new Map(); const ignoredDirectorySet = new Set(ignoredDirectories); for (const entry of readdirSync(testsDir, { withFileTypes: true })) { @@ -187,6 +307,7 @@ function checkExplicitIntegrationTestTopology(root, { join(testsDir, entry.name), testsDir, leafRustFiles, + leafSources, ignoredDirectorySet, ); } @@ -199,6 +320,8 @@ function checkExplicitIntegrationTestTopology(root, { topLevelRustFiles, rootSources, leafRustFiles, + leafSources, + allowedLeafNegativeFeatures, }).map((message) => ({ path: manifestPath, line: 1, message })); } @@ -216,3 +339,28 @@ export function checkCliIntegrationTestTopology(root) { ignoredDirectories: ['tests/support'], }); } + +export function checkServicesCoreIntegrationTestTopology(root) { + return checkExplicitIntegrationTestTopology(root, { + cratePath: 'src/crates/services/services-core', + expectedTargets: servicesCoreIntegrationTestTargets, + }); +} + +export function checkServicesIntegrationsIntegrationTestTopology(root) { + return checkExplicitIntegrationTestTopology(root, { + cratePath: 'src/crates/services/services-integrations', + expectedTargets: servicesIntegrationsIntegrationTestTargets, + allowedLeafNegativeFeatures: new Map([[ + 'tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs', + new Set(['remote-ssh-concrete']), + ]]), + }); +} + +export function checkServiceIntegrationTestTopologies(root) { + return [ + ...checkServicesCoreIntegrationTestTopology(root), + ...checkServicesIntegrationsIntegrationTestTopology(root), + ]; +} diff --git a/scripts/core-boundaries/rules/source/required-rules.mjs b/scripts/core-boundaries/rules/source/required-rules.mjs index 774928a2d..abe95da81 100644 --- a/scripts/core-boundaries/rules/source/required-rules.mjs +++ b/scripts/core-boundaries/rules/source/required-rules.mjs @@ -230,7 +230,7 @@ export const requiredContentRules = [ ], }, { - path: 'src/crates/services/services-core/tests/storage_owner_contracts.rs', + path: 'src/crates/services/services-core/tests/storage_owner_contracts/storage_owner_contracts.rs', reason: 'services-core local storage owner must keep persistence, cleanup, and token usage behavior contracts', patterns: [ @@ -290,6 +290,18 @@ export const requiredContentRules = [ }, ], }, + { + path: 'src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs', + reason: + 'remote SSH disabled contracts must stay excluded when the concrete provider is enabled', + patterns: [ + { + regex: + /^#!\[cfg\(all\(feature = "remote-ssh", not\(feature = "remote-ssh-concrete"\)\)\)\]$/m, + message: 'missing remote SSH concrete-disabled feature gate', + }, + ], + }, { path: 'src/crates/assembly/core/src/infrastructure/debug_log/mod.rs', reason: diff --git a/scripts/core-boundaries/self-test.mjs b/scripts/core-boundaries/self-test.mjs index 24a30dfac..4200b2c19 100644 --- a/scripts/core-boundaries/self-test.mjs +++ b/scripts/core-boundaries/self-test.mjs @@ -80,6 +80,7 @@ export function runManifestParserSelfTest({ topLevelRustFiles: agentRuntimeIntegrationTestTargets.map(({ path }) => path), rootSources: explicitTestRoots, leafRustFiles: ['tests/agent_definition_contracts/prompt_contracts.rs'], + leafSources: new Map([['tests/agent_definition_contracts/prompt_contracts.rs', '']]), }; const topologyErrors = validateExplicitIntegrationTestTopology(explicitTestFixture); if (topologyErrors.length > 0) { @@ -95,6 +96,162 @@ export function runManifestParserSelfTest({ if (!orphanErrors.some((error) => error.includes('orphan_contracts.rs'))) { throw new Error('explicit integration-test topology must reject an orphan leaf test'); } + const extraFeatureLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(feature = "network-transport")]\n', + ]]), + }); + if (!extraFeatureLeafErrors.some((error) => error.includes('network-transport'))) { + throw new Error('grouped test topology must reject a leaf with an extra positive feature gate'); + } + const platformLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(windows)]\n', + ]]), + }); + if (!platformLeafErrors.some((error) => error.includes('platform'))) { + throw new Error('grouped test topology must reject a leaf-level platform gate'); + } + const cfgAttrLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg_attr(feature = "product-full", cfg(windows))]\n', + ]]), + }); + if (!cfgAttrLeafErrors.some((error) => error.includes('cfg_attr'))) { + throw new Error('grouped test topology must reject a leaf-level cfg_attr gate'); + } + const spacedCfgAttrLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '# ! [cfg_attr(feature = "product-full", cfg(windows))]\n', + ]]), + }); + if (!spacedCfgAttrLeafErrors.some((error) => error.includes('cfg_attr'))) { + throw new Error('grouped test topology must reject a token-spaced crate-level cfg_attr gate'); + } + const commentedCfgAttrLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#/**/!/**/[/**/cfg_attr(feature = "product-full", cfg(windows))]\n', + ]]), + }); + if (!commentedCfgAttrLeafErrors.some((error) => error.includes('cfg_attr'))) { + throw new Error('grouped test topology must reject a comment-spaced crate-level cfg_attr gate'); + } + const bracketCommentLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![/* ] */ cfg(feature = "network-transport")]\n', + ]]), + }); + if (!bracketCommentLeafErrors.some((error) => error.includes('network-transport'))) { + throw new Error('grouped test topology must ignore closing brackets inside comments'); + } + const parenthesisCommentLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(all(/* ) */ feature = "network-transport"))]\n', + ]]), + }); + if (!parenthesisCommentLeafErrors.some((error) => error.includes('network-transport'))) { + throw new Error('grouped test topology must ignore closing parentheses inside comments'); + } + const rawDocCfgAttrLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![doc = r#"a"]b"#]\n#![cfg_attr(feature = "product-full", cfg(windows))]\n', + ]]), + }); + if (!rawDocCfgAttrLeafErrors.some((error) => error.includes('cfg_attr'))) { + throw new Error('grouped test topology must ignore delimiters inside raw string literals'); + } + const rawIdentifierCfgAttrLeafErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![r#cfg_attr(feature = "product-full", cfg(windows))]\n', + ]]), + }); + if (!rawIdentifierCfgAttrLeafErrors.some((error) => error.includes('cfg_attr'))) { + throw new Error('grouped test topology must normalize raw cfg_attr identifiers'); + } + const multilineLeafCfgErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(\n all(\n feature = "network-transport",\n windows,\n )\n)]\n', + ]]), + }); + if (!multilineLeafCfgErrors.some((error) => error.includes('network-transport'))) { + throw new Error('grouped test topology must parse multiline crate cfg attributes'); + } + const negatedRootFeatureErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + rootSources: new Map([ + ...explicitTestRoots, + [ + 'tests/agent_definition_contracts.rs', + '#![cfg(feature = "agent-definitions")]\n#[path = "agent_definition_contracts/prompt_contracts.rs"]\nmod prompt_contracts;', + ], + ]), + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(not(feature = "agent-definitions"))]\n', + ]]), + }); + if (!negatedRootFeatureErrors.some((error) => error.includes('negates root feature'))) { + throw new Error('grouped test topology must reject a leaf that negates a root feature'); + } + const unrelatedNegativeFeatureErrors = validateExplicitIntegrationTestTopology({ + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(not(feature = "unrelated-feature"))]\n', + ]]), + }); + if (!unrelatedNegativeFeatureErrors.some((error) => error.includes('unreviewed negative feature'))) { + throw new Error('grouped test topology must reject an unreviewed negative feature gate'); + } + const reviewedNegativeFeatureFixture = { + ...explicitTestFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(not(feature = "reviewed-negative"))]\n', + ]]), + allowedLeafNegativeFeatures: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + new Set(['reviewed-negative']), + ]]), + }; + const reviewedNegativeFeatureErrors = validateExplicitIntegrationTestTopology( + reviewedNegativeFeatureFixture, + ); + if (reviewedNegativeFeatureErrors.length > 0) { + throw new Error( + `grouped test topology rejected a reviewed negative feature: ${reviewedNegativeFeatureErrors.join('; ')}`, + ); + } + const extraNegativeFeatureErrors = validateExplicitIntegrationTestTopology({ + ...reviewedNegativeFeatureFixture, + leafSources: new Map([[ + 'tests/agent_definition_contracts/prompt_contracts.rs', + '#![cfg(not(feature = "reviewed-negative"))]\n#![cfg(not(feature = "unrelated-feature"))]\n', + ]]), + }); + if (!extraNegativeFeatureErrors.some((error) => error.includes('unrelated-feature'))) { + throw new Error('grouped test topology must reject extra negative features on a reviewed leaf'); + } const wrongSectionErrors = validateExplicitIntegrationTestTopology({ ...explicitTestFixture, manifestText: explicitTestManifest.replace( @@ -4884,6 +5041,21 @@ async fn release_baseline_claim(release: BaselineClaimRelease) -> Result<(), Dis throw new Error('no-default dispatch claim release rule must reject a discarded error followed by success'); } + const remoteSshDisabledCfgRule = requiredContentRules + .find((rule) => rule.path === 'src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs') + ?.patterns.find((pattern) => pattern.message === 'missing remote SSH concrete-disabled feature gate'); + if (!remoteSshDisabledCfgRule) { + throw new Error('missing remote SSH concrete-disabled feature gate rule'); + } + if (!remoteSshDisabledCfgRule.regex.test( + '#![cfg(all(feature = "remote-ssh", not(feature = "remote-ssh-concrete")))]', + )) { + throw new Error('remote SSH concrete-disabled feature gate rule must accept the reviewed cfg'); + } + if (remoteSshDisabledCfgRule.regex.test('#![cfg(feature = "remote-ssh")]')) { + throw new Error('remote SSH concrete-disabled feature gate rule must reject a missing negative gate'); + } + const sessionControlRuleText = forbiddenRuleTextForPath( 'src/crates/assembly/core/src/agentic/tools/implementations/session_control_tool.rs', ); diff --git a/src/crates/services/services-core/AGENTS.md b/src/crates/services/services-core/AGENTS.md index 87749c8a2..ae34edff6 100644 --- a/src/crates/services/services-core/AGENTS.md +++ b/src/crates/services/services-core/AGENTS.md @@ -61,18 +61,19 @@ crate. ## Verification +Start from the capability that owns the change. Integration targets group test +source files with the same owner and feature closure; keep a focused run small +with `--test ::` instead of adding another Cargo +target. Representative stable entry points are: + ```bash cargo check -p bitfun-services-core --no-default-features cargo check -p bitfun-services-core --no-default-features --features filesystem -cargo test -p bitfun-services-core --no-default-features --features json-io --lib json_store -cargo test -p bitfun-services-core --no-default-features --features local-storage --test session_metadata_contracts +cargo test -p bitfun-services-core --no-default-features --features local-storage --test session_contracts session_metadata_contracts:: +cargo test -p bitfun-services-core --no-default-features --features local-storage --test session_write_lock_contracts cargo test -p bitfun-services-core --no-default-features --features process-runtime --test process_runtime_contracts -cargo test -p bitfun-services-core --no-default-features --features workspace-instructions --test declarative_workspace_instruction_contracts -cargo test -p bitfun-services-core --no-default-features --features lsp --test lsp_plugin_registry_contracts -cargo test -p bitfun-services-core --no-default-features --features session-git memory_workspace -cargo check -p bitfun-services-core --no-default-features --features workspace-identity -cargo test -p bitfun-services-core --no-default-features --features workspace-runtime workspace -cargo test -p bitfun-services-core --no-default-features --features runtime-ownership --test runtime_ownership_contracts -node scripts/check-core-boundaries.mjs -cargo check -p bitfun-core --features product-full +pnpm run check:core-boundaries ``` + +Other capability-specific target names remain in `Cargo.toml`; document a new +command here only when it becomes a recurring owner workflow. diff --git a/src/crates/services/services-core/Cargo.toml b/src/crates/services/services-core/Cargo.toml index 45e9a63f6..253487755 100644 --- a/src/crates/services/services-core/Cargo.toml +++ b/src/crates/services/services-core/Cargo.toml @@ -4,6 +4,7 @@ version.workspace = true authors.workspace = true edition.workspace = true description = "BitFun core service owner crate" +autotests = false [lib] name = "bitfun_services_core" @@ -117,70 +118,66 @@ tokio = { workspace = true, features = ["macros"] } [[test]] name = "markdown_owner_contracts" +path = "tests/markdown_owner_contracts.rs" required-features = ["markdown"] [[test]] name = "declarative_workspace_instruction_contracts" +path = "tests/declarative_workspace_instruction_contracts.rs" required-features = ["workspace-instructions"] -[[test]] -name = "json_store_contracts" -required-features = ["local-storage"] - [[test]] name = "lsp_plugin_registry_contracts" +path = "tests/lsp_plugin_registry_contracts.rs" required-features = ["lsp"] [[test]] name = "runtime_ownership_contracts" +path = "tests/runtime_ownership_contracts.rs" required-features = ["runtime-ownership"] [[test]] name = "local_runtime_ports" +path = "tests/local_runtime_ports.rs" required-features = ["workspace-runtime"] [[test]] name = "permission_store_contracts" +path = "tests/permission_store_contracts.rs" required-features = ["permission"] [[test]] name = "workspace_instruction_contracts" +path = "tests/workspace_instruction_contracts.rs" required-features = ["workspace-instructions", "workspace-runtime"] [[test]] name = "session_write_lock_contracts" +path = "tests/session_write_lock_contracts.rs" required-features = ["local-storage"] [[test]] name = "process_runtime_contracts" +path = "tests/process_runtime_contracts.rs" required-features = ["process-runtime"] [[test]] -name = "session_contracts" -required-features = ["local-storage"] +name = "service_contracts" +path = "tests/service_contracts.rs" [[test]] -name = "session_layout_contracts" -required-features = ["local-storage"] - -[[test]] -name = "session_metadata_contracts" +name = "storage_owner_contracts" +path = "tests/storage_owner_contracts.rs" required-features = ["local-storage"] [[test]] -name = "session_page_contracts" +name = "session_contracts" +path = "tests/session_contracts.rs" required-features = ["local-storage"] [[test]] name = "session_usage_contracts" -required-features = ["local-storage"] - -[[test]] -name = "storage_owner_contracts" -required-features = ["local-storage"] - -[[test]] -name = "token_usage_contracts" +path = "tests/session_usage_contracts.rs" required-features = ["local-storage"] [lints] diff --git a/src/crates/services/services-core/tests/service_contracts.rs b/src/crates/services/services-core/tests/service_contracts.rs index 26d293f8b..a105a8665 100644 --- a/src/crates/services/services-core/tests/service_contracts.rs +++ b/src/crates/services/services-core/tests/service_contracts.rs @@ -1,17 +1,6 @@ -use bitfun_services_core::diff::{DiffConfig, DiffLineType, DiffService}; - -#[test] -fn diff_service_preserves_line_count_contract() { - let service = DiffService::new(DiffConfig::new()); - let result = service.compute_diff("one\ntwo\n", "one\nthree\n"); - - assert_eq!(result.additions, 1); - assert_eq!(result.deletions, 1); - assert_eq!(result.changes, 2); - assert_eq!(result.hunks.len(), 1); - assert!(result - .hunks - .iter() - .flat_map(|hunk| hunk.lines.iter()) - .any(|line| line.line_type == DiffLineType::Add && line.content == "three")); -} +#[path = "service_contracts/diagnostic_log_redaction.rs"] +mod diagnostic_log_redaction; +#[path = "service_contracts/jsonc_contracts.rs"] +mod jsonc_contracts; +#[path = "service_contracts/service_contracts.rs"] +mod service_contracts; diff --git a/src/crates/services/services-core/tests/diagnostic_log_redaction.rs b/src/crates/services/services-core/tests/service_contracts/diagnostic_log_redaction.rs similarity index 100% rename from src/crates/services/services-core/tests/diagnostic_log_redaction.rs rename to src/crates/services/services-core/tests/service_contracts/diagnostic_log_redaction.rs diff --git a/src/crates/services/services-core/tests/jsonc_contracts.rs b/src/crates/services/services-core/tests/service_contracts/jsonc_contracts.rs similarity index 100% rename from src/crates/services/services-core/tests/jsonc_contracts.rs rename to src/crates/services/services-core/tests/service_contracts/jsonc_contracts.rs diff --git a/src/crates/services/services-core/tests/service_contracts/service_contracts.rs b/src/crates/services/services-core/tests/service_contracts/service_contracts.rs new file mode 100644 index 000000000..26d293f8b --- /dev/null +++ b/src/crates/services/services-core/tests/service_contracts/service_contracts.rs @@ -0,0 +1,17 @@ +use bitfun_services_core::diff::{DiffConfig, DiffLineType, DiffService}; + +#[test] +fn diff_service_preserves_line_count_contract() { + let service = DiffService::new(DiffConfig::new()); + let result = service.compute_diff("one\ntwo\n", "one\nthree\n"); + + assert_eq!(result.additions, 1); + assert_eq!(result.deletions, 1); + assert_eq!(result.changes, 2); + assert_eq!(result.hunks.len(), 1); + assert!(result + .hunks + .iter() + .flat_map(|hunk| hunk.lines.iter()) + .any(|line| line.line_type == DiffLineType::Add && line.content == "three")); +} diff --git a/src/crates/services/services-core/tests/session_contracts.rs b/src/crates/services/services-core/tests/session_contracts.rs index 1ad2858a2..7d8eb66a9 100644 --- a/src/crates/services/services-core/tests/session_contracts.rs +++ b/src/crates/services/services-core/tests/session_contracts.rs @@ -1,40 +1,10 @@ #![cfg(feature = "local-storage")] -use bitfun_services_core::session::{DialogTurnKind, SessionKind, SessionMetadata}; - -#[test] -fn session_metadata_preserves_subagent_visibility_contract() { - let mut metadata = SessionMetadata::new( - "session-1".to_string(), - "Subagent: inspect".to_string(), - "Explore".to_string(), - "model".to_string(), - ); - metadata.session_kind = SessionKind::Subagent; - - assert!(metadata.is_subagent()); - assert!(metadata.should_hide_from_user_lists()); -} - -#[test] -fn session_metadata_hides_ephemeral_child_sessions_from_user_lists() { - let mut metadata = SessionMetadata::new( - "session-ephemeral".to_string(), - "Side thread".to_string(), - "agentic".to_string(), - "model".to_string(), - ); - metadata.session_kind = SessionKind::EphemeralChild; - - assert!(!metadata.is_subagent()); - assert!(metadata.is_internal_hidden()); - assert!(metadata.should_hide_from_user_lists()); -} - -#[test] -fn dialog_turn_kind_preserves_default_visibility_contract() { - assert_eq!(DialogTurnKind::default(), DialogTurnKind::UserDialog); - assert!(DialogTurnKind::UserDialog.is_model_visible()); - assert!(!DialogTurnKind::ManualCompaction.is_model_visible()); - assert!(!DialogTurnKind::LocalCommand.is_model_visible()); -} +#[path = "session_contracts/session_contracts.rs"] +mod session_contracts; +#[path = "session_contracts/session_layout_contracts.rs"] +mod session_layout_contracts; +#[path = "session_contracts/session_metadata_contracts.rs"] +mod session_metadata_contracts; +#[path = "session_contracts/session_page_contracts.rs"] +mod session_page_contracts; diff --git a/src/crates/services/services-core/tests/session_contracts/session_contracts.rs b/src/crates/services/services-core/tests/session_contracts/session_contracts.rs new file mode 100644 index 000000000..1ad2858a2 --- /dev/null +++ b/src/crates/services/services-core/tests/session_contracts/session_contracts.rs @@ -0,0 +1,40 @@ +#![cfg(feature = "local-storage")] + +use bitfun_services_core::session::{DialogTurnKind, SessionKind, SessionMetadata}; + +#[test] +fn session_metadata_preserves_subagent_visibility_contract() { + let mut metadata = SessionMetadata::new( + "session-1".to_string(), + "Subagent: inspect".to_string(), + "Explore".to_string(), + "model".to_string(), + ); + metadata.session_kind = SessionKind::Subagent; + + assert!(metadata.is_subagent()); + assert!(metadata.should_hide_from_user_lists()); +} + +#[test] +fn session_metadata_hides_ephemeral_child_sessions_from_user_lists() { + let mut metadata = SessionMetadata::new( + "session-ephemeral".to_string(), + "Side thread".to_string(), + "agentic".to_string(), + "model".to_string(), + ); + metadata.session_kind = SessionKind::EphemeralChild; + + assert!(!metadata.is_subagent()); + assert!(metadata.is_internal_hidden()); + assert!(metadata.should_hide_from_user_lists()); +} + +#[test] +fn dialog_turn_kind_preserves_default_visibility_contract() { + assert_eq!(DialogTurnKind::default(), DialogTurnKind::UserDialog); + assert!(DialogTurnKind::UserDialog.is_model_visible()); + assert!(!DialogTurnKind::ManualCompaction.is_model_visible()); + assert!(!DialogTurnKind::LocalCommand.is_model_visible()); +} diff --git a/src/crates/services/services-core/tests/session_layout_contracts.rs b/src/crates/services/services-core/tests/session_contracts/session_layout_contracts.rs similarity index 100% rename from src/crates/services/services-core/tests/session_layout_contracts.rs rename to src/crates/services/services-core/tests/session_contracts/session_layout_contracts.rs diff --git a/src/crates/services/services-core/tests/session_metadata_contracts.rs b/src/crates/services/services-core/tests/session_contracts/session_metadata_contracts.rs similarity index 100% rename from src/crates/services/services-core/tests/session_metadata_contracts.rs rename to src/crates/services/services-core/tests/session_contracts/session_metadata_contracts.rs diff --git a/src/crates/services/services-core/tests/session_page_contracts.rs b/src/crates/services/services-core/tests/session_contracts/session_page_contracts.rs similarity index 100% rename from src/crates/services/services-core/tests/session_page_contracts.rs rename to src/crates/services/services-core/tests/session_contracts/session_page_contracts.rs diff --git a/src/crates/services/services-core/tests/session_usage_contracts.rs b/src/crates/services/services-core/tests/session_usage_contracts.rs index bacdc5819..d3b2a1f31 100644 --- a/src/crates/services/services-core/tests/session_usage_contracts.rs +++ b/src/crates/services/services-core/tests/session_usage_contracts.rs @@ -1,36 +1,6 @@ #![cfg(feature = "local-storage")] -use bitfun_services_core::session_usage::{ - classify_tool_usage, display_workspace_relative_path, render_usage_report_terminal, - SessionUsageReport, UsageToolCategory, -}; - -#[test] -fn usage_classifier_preserves_git_command_detection() { - let input = serde_json::json!({ "command": "git status --short" }); - - assert_eq!( - classify_tool_usage("execute_command", Some(&input)), - UsageToolCategory::Git - ); -} - -#[test] -fn usage_path_redaction_preserves_workspace_relative_display() { - let label = display_workspace_relative_path( - Some("D:/workspace/bitfun"), - "D:/workspace/bitfun/src/main.rs", - ); - - assert_eq!(label.value, "src/main.rs"); - assert!(!label.redacted); -} - -#[test] -fn usage_terminal_renderer_preserves_schema_label() { - let report = SessionUsageReport::partial_unavailable("session-1".to_string(), 42); - - let rendered = render_usage_report_terminal(&report); - - assert!(rendered.contains("session-1")); -} +#[path = "session_usage_contracts/session_usage_contracts.rs"] +mod session_usage_contracts; +#[path = "session_usage_contracts/token_usage_contracts.rs"] +mod token_usage_contracts; diff --git a/src/crates/services/services-core/tests/session_usage_contracts/session_usage_contracts.rs b/src/crates/services/services-core/tests/session_usage_contracts/session_usage_contracts.rs new file mode 100644 index 000000000..bacdc5819 --- /dev/null +++ b/src/crates/services/services-core/tests/session_usage_contracts/session_usage_contracts.rs @@ -0,0 +1,36 @@ +#![cfg(feature = "local-storage")] + +use bitfun_services_core::session_usage::{ + classify_tool_usage, display_workspace_relative_path, render_usage_report_terminal, + SessionUsageReport, UsageToolCategory, +}; + +#[test] +fn usage_classifier_preserves_git_command_detection() { + let input = serde_json::json!({ "command": "git status --short" }); + + assert_eq!( + classify_tool_usage("execute_command", Some(&input)), + UsageToolCategory::Git + ); +} + +#[test] +fn usage_path_redaction_preserves_workspace_relative_display() { + let label = display_workspace_relative_path( + Some("D:/workspace/bitfun"), + "D:/workspace/bitfun/src/main.rs", + ); + + assert_eq!(label.value, "src/main.rs"); + assert!(!label.redacted); +} + +#[test] +fn usage_terminal_renderer_preserves_schema_label() { + let report = SessionUsageReport::partial_unavailable("session-1".to_string(), 42); + + let rendered = render_usage_report_terminal(&report); + + assert!(rendered.contains("session-1")); +} diff --git a/src/crates/services/services-core/tests/token_usage_contracts.rs b/src/crates/services/services-core/tests/session_usage_contracts/token_usage_contracts.rs similarity index 100% rename from src/crates/services/services-core/tests/token_usage_contracts.rs rename to src/crates/services/services-core/tests/session_usage_contracts/token_usage_contracts.rs diff --git a/src/crates/services/services-core/tests/storage_owner_contracts.rs b/src/crates/services/services-core/tests/storage_owner_contracts.rs index bf03c4ceb..4db583a10 100644 --- a/src/crates/services/services-core/tests/storage_owner_contracts.rs +++ b/src/crates/services/services-core/tests/storage_owner_contracts.rs @@ -1,342 +1,6 @@ #![cfg(feature = "local-storage")] -use bitfun_services_core::persistence::{PersistenceService, StorageOptions}; -use bitfun_services_core::storage_cleanup::{CleanupPolicy, CleanupRoots, CleanupService}; -use serde::{Deserialize, Serialize}; -use serde_json::json; -use std::fs; -use std::time::{Duration, SystemTime}; - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -struct DemoRecord { - name: String, - count: u32, -} - -#[tokio::test] -async fn persistence_service_keeps_atomic_json_shape_and_backups() { - let temp = tempfile::tempdir().expect("tempdir"); - let service = PersistenceService::new(temp.path().join("store")) - .await - .expect("service"); - - service - .save_json( - "demo", - &DemoRecord { - name: "first".to_string(), - count: 1, - }, - StorageOptions::default(), - ) - .await - .expect("first save"); - service - .save_json( - "demo", - &DemoRecord { - name: "second".to_string(), - count: 2, - }, - StorageOptions::default(), - ) - .await - .expect("second save"); - - let loaded: DemoRecord = service - .load_json("demo") - .await - .expect("load") - .expect("record"); - assert_eq!( - loaded, - DemoRecord { - name: "second".to_string(), - count: 2 - } - ); - - let backups = fs::read_dir(temp.path().join("store").join("backups")) - .expect("backup dir") - .count(); - assert_eq!(backups, 1); - assert!(service.delete("demo").await.expect("delete")); - assert!(service - .load_json::("demo") - .await - .expect("load missing") - .is_none()); -} - -#[tokio::test] -async fn cleanup_service_deletes_old_temp_and_log_files_without_product_paths() { - let temp = tempfile::tempdir().expect("tempdir"); - let temp_dir = temp.path().join("temp"); - let logs_dir = temp.path().join("logs"); - let cache_dir = temp.path().join("cache"); - fs::create_dir_all(&temp_dir).expect("temp dir"); - fs::create_dir_all(&logs_dir).expect("logs dir"); - fs::create_dir_all(&cache_dir).expect("cache dir"); - - let old_temp_file = temp_dir.join("old.tmp"); - let old_log_file = logs_dir.join("old.log"); - fs::write(&old_temp_file, "old temp").expect("old temp"); - fs::write(&old_log_file, "old log").expect("old log"); - - let old_time = filetime::FileTime::from_system_time( - SystemTime::now() - Duration::from_secs(60 * 60 * 24 * 10), - ); - filetime::set_file_mtime(&old_temp_file, old_time).expect("mtime temp"); - filetime::set_file_mtime(&old_log_file, old_time).expect("mtime log"); - - let service = CleanupService::new( - CleanupRoots { - temp_dir, - logs_dir, - cache_dir, - }, - CleanupPolicy { - temp_retention_days: 7, - log_retention_days: 7, - ..CleanupPolicy::default() - }, - ); - - let result = service.cleanup_all().await.expect("cleanup"); - assert_eq!(result.files_deleted, 2); - assert!(!old_temp_file.exists()); - assert!(!old_log_file.exists()); -} - -#[tokio::test] -async fn cleanup_service_trims_oldest_cache_files_when_size_exceeds_policy() { - let temp = tempfile::tempdir().expect("tempdir"); - let temp_dir = temp.path().join("temp"); - let logs_dir = temp.path().join("logs"); - let cache_dir = temp.path().join("cache"); - fs::create_dir_all(&temp_dir).expect("temp dir"); - fs::create_dir_all(&logs_dir).expect("logs dir"); - fs::create_dir_all(&cache_dir).expect("cache dir"); - - let newest_file = cache_dir.join("newest.bin"); - let middle_file = cache_dir.join("middle.bin"); - let oldest_file = cache_dir.join("oldest.bin"); - let two_mb = vec![b'x'; 2 * 1_048_576]; - fs::write(&newest_file, &two_mb).expect("newest"); - fs::write(&middle_file, &two_mb).expect("middle"); - fs::write(&oldest_file, &two_mb).expect("oldest"); - - let now = SystemTime::now(); - filetime::set_file_mtime( - &newest_file, - filetime::FileTime::from_system_time(now - Duration::from_secs(60)), - ) - .expect("newest mtime"); - filetime::set_file_mtime( - &middle_file, - filetime::FileTime::from_system_time(now - Duration::from_secs(120)), - ) - .expect("middle mtime"); - filetime::set_file_mtime( - &oldest_file, - filetime::FileTime::from_system_time(now - Duration::from_secs(180)), - ) - .expect("oldest mtime"); - - let service = CleanupService::new( - CleanupRoots { - temp_dir, - logs_dir, - cache_dir, - }, - CleanupPolicy { - max_cache_size_mb: 4, - ..CleanupPolicy::default() - }, - ); - - let result = service.cleanup_all().await.expect("cleanup"); - - assert_eq!(result.files_deleted, 1); - assert_eq!(result.bytes_freed, two_mb.len() as u64); - assert!(newest_file.exists()); - assert!(middle_file.exists()); - assert!(!oldest_file.exists()); - assert_eq!(result.categories.len(), 1); - assert_eq!(result.categories[0].name, "Oversized Cache"); -} - -#[tokio::test] -async fn token_usage_service_persists_records_and_filters_subagents_by_default() { - let temp = tempfile::tempdir().expect("tempdir"); - let service = - bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) - .await - .expect("service"); - - service - .record_usage( - "model-config-a".to_string(), - "model-a".to_string(), - "session-a".to_string(), - "turn-a".to_string(), - 100, - 40, - Some(30), - Some(json!({ "cacheCreationTokenCount": 12 })), - false, - ) - .await - .expect("record main"); - service - .record_usage( - "model-config-a".to_string(), - "model-a".to_string(), - "session-a".to_string(), - "turn-sub".to_string(), - 50, - 10, - None, - None, - true, - ) - .await - .expect("record subagent"); - - let summary = service - .get_summary(bitfun_services_core::token_usage::TokenUsageQuery { - model_id: Some("model-a".to_string()), - session_id: None, - time_range: bitfun_services_core::token_usage::TimeRange::All, - limit: None, - offset: None, - include_subagent: false, - }) - .await - .expect("summary"); - - assert_eq!(summary.record_count, 1); - assert_eq!(summary.total_input, 100); - assert_eq!(summary.total_cached, 30); - assert_eq!(summary.total_cache_write, 12); - - let reloaded = - bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) - .await - .expect("reloaded"); - let stats = reloaded - .get_model_stats("model-a") - .await - .expect("model stats"); - assert_eq!(stats.request_count, 2); - assert_eq!(stats.total_input, 150); -} - -#[tokio::test] -async fn token_usage_clear_does_not_replay_cached_record_batches() { - let temp = tempfile::tempdir().expect("tempdir"); - let service = - bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) - .await - .expect("service"); - - service - .record_usage( - "model-config-old".to_string(), - "model-old".to_string(), - "session-old".to_string(), - "turn-old".to_string(), - 10, - 5, - None, - None, - false, - ) - .await - .expect("record old usage"); - service.clear_all_stats().await.expect("clear usage"); - service - .record_usage( - "model-config-new".to_string(), - "model-new".to_string(), - "session-new".to_string(), - "turn-new".to_string(), - 20, - 7, - None, - None, - false, - ) - .await - .expect("record new usage"); - - let summary = service - .get_summary(bitfun_services_core::token_usage::TokenUsageQuery { - model_id: None, - session_id: None, - time_range: bitfun_services_core::token_usage::TimeRange::All, - limit: None, - offset: None, - include_subagent: true, - }) - .await - .expect("summary after clear"); - - assert_eq!(summary.record_count, 1); - assert_eq!(summary.total_input, 20); - assert!(service.get_model_stats("model-old").await.is_none()); - assert_eq!( - service - .get_model_stats("model-new") - .await - .expect("new model stats") - .request_count, - 1 - ); -} - -#[tokio::test] -async fn token_usage_all_range_ignores_non_date_record_files() { - let temp = tempfile::tempdir().expect("tempdir"); - let service = - bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) - .await - .expect("service"); - - service - .record_usage( - "model-config-a".to_string(), - "model-a".to_string(), - "session-a".to_string(), - "turn-a".to_string(), - 10, - 1, - None, - None, - false, - ) - .await - .expect("record usage"); - - let records_dir = temp.path().join("records"); - fs::write( - records_dir.join("manual-backup.json"), - r#"{"records":[{"model_id":"model-b","session_id":"session-b","turn_id":"turn-b","timestamp":"2026-07-07T00:00:00Z","input_tokens":999,"output_tokens":1,"cached_tokens":0,"cached_tokens_available":false,"cache_write_tokens":0,"total_tokens":1000,"token_details":null,"is_subagent":false}]}"#, - ) - .expect("write stray record file"); - - let summary = service - .get_summary(bitfun_services_core::token_usage::TokenUsageQuery { - model_id: None, - session_id: None, - time_range: bitfun_services_core::token_usage::TimeRange::All, - limit: None, - offset: None, - include_subagent: true, - }) - .await - .expect("summary"); - - assert_eq!(summary.record_count, 1); - assert_eq!(summary.total_input, 10); -} +#[path = "storage_owner_contracts/json_store_contracts.rs"] +mod json_store_contracts; +#[path = "storage_owner_contracts/storage_owner_contracts.rs"] +mod storage_owner_contracts; diff --git a/src/crates/services/services-core/tests/json_store_contracts.rs b/src/crates/services/services-core/tests/storage_owner_contracts/json_store_contracts.rs similarity index 100% rename from src/crates/services/services-core/tests/json_store_contracts.rs rename to src/crates/services/services-core/tests/storage_owner_contracts/json_store_contracts.rs diff --git a/src/crates/services/services-core/tests/storage_owner_contracts/storage_owner_contracts.rs b/src/crates/services/services-core/tests/storage_owner_contracts/storage_owner_contracts.rs new file mode 100644 index 000000000..bf03c4ceb --- /dev/null +++ b/src/crates/services/services-core/tests/storage_owner_contracts/storage_owner_contracts.rs @@ -0,0 +1,342 @@ +#![cfg(feature = "local-storage")] + +use bitfun_services_core::persistence::{PersistenceService, StorageOptions}; +use bitfun_services_core::storage_cleanup::{CleanupPolicy, CleanupRoots, CleanupService}; +use serde::{Deserialize, Serialize}; +use serde_json::json; +use std::fs; +use std::time::{Duration, SystemTime}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct DemoRecord { + name: String, + count: u32, +} + +#[tokio::test] +async fn persistence_service_keeps_atomic_json_shape_and_backups() { + let temp = tempfile::tempdir().expect("tempdir"); + let service = PersistenceService::new(temp.path().join("store")) + .await + .expect("service"); + + service + .save_json( + "demo", + &DemoRecord { + name: "first".to_string(), + count: 1, + }, + StorageOptions::default(), + ) + .await + .expect("first save"); + service + .save_json( + "demo", + &DemoRecord { + name: "second".to_string(), + count: 2, + }, + StorageOptions::default(), + ) + .await + .expect("second save"); + + let loaded: DemoRecord = service + .load_json("demo") + .await + .expect("load") + .expect("record"); + assert_eq!( + loaded, + DemoRecord { + name: "second".to_string(), + count: 2 + } + ); + + let backups = fs::read_dir(temp.path().join("store").join("backups")) + .expect("backup dir") + .count(); + assert_eq!(backups, 1); + assert!(service.delete("demo").await.expect("delete")); + assert!(service + .load_json::("demo") + .await + .expect("load missing") + .is_none()); +} + +#[tokio::test] +async fn cleanup_service_deletes_old_temp_and_log_files_without_product_paths() { + let temp = tempfile::tempdir().expect("tempdir"); + let temp_dir = temp.path().join("temp"); + let logs_dir = temp.path().join("logs"); + let cache_dir = temp.path().join("cache"); + fs::create_dir_all(&temp_dir).expect("temp dir"); + fs::create_dir_all(&logs_dir).expect("logs dir"); + fs::create_dir_all(&cache_dir).expect("cache dir"); + + let old_temp_file = temp_dir.join("old.tmp"); + let old_log_file = logs_dir.join("old.log"); + fs::write(&old_temp_file, "old temp").expect("old temp"); + fs::write(&old_log_file, "old log").expect("old log"); + + let old_time = filetime::FileTime::from_system_time( + SystemTime::now() - Duration::from_secs(60 * 60 * 24 * 10), + ); + filetime::set_file_mtime(&old_temp_file, old_time).expect("mtime temp"); + filetime::set_file_mtime(&old_log_file, old_time).expect("mtime log"); + + let service = CleanupService::new( + CleanupRoots { + temp_dir, + logs_dir, + cache_dir, + }, + CleanupPolicy { + temp_retention_days: 7, + log_retention_days: 7, + ..CleanupPolicy::default() + }, + ); + + let result = service.cleanup_all().await.expect("cleanup"); + assert_eq!(result.files_deleted, 2); + assert!(!old_temp_file.exists()); + assert!(!old_log_file.exists()); +} + +#[tokio::test] +async fn cleanup_service_trims_oldest_cache_files_when_size_exceeds_policy() { + let temp = tempfile::tempdir().expect("tempdir"); + let temp_dir = temp.path().join("temp"); + let logs_dir = temp.path().join("logs"); + let cache_dir = temp.path().join("cache"); + fs::create_dir_all(&temp_dir).expect("temp dir"); + fs::create_dir_all(&logs_dir).expect("logs dir"); + fs::create_dir_all(&cache_dir).expect("cache dir"); + + let newest_file = cache_dir.join("newest.bin"); + let middle_file = cache_dir.join("middle.bin"); + let oldest_file = cache_dir.join("oldest.bin"); + let two_mb = vec![b'x'; 2 * 1_048_576]; + fs::write(&newest_file, &two_mb).expect("newest"); + fs::write(&middle_file, &two_mb).expect("middle"); + fs::write(&oldest_file, &two_mb).expect("oldest"); + + let now = SystemTime::now(); + filetime::set_file_mtime( + &newest_file, + filetime::FileTime::from_system_time(now - Duration::from_secs(60)), + ) + .expect("newest mtime"); + filetime::set_file_mtime( + &middle_file, + filetime::FileTime::from_system_time(now - Duration::from_secs(120)), + ) + .expect("middle mtime"); + filetime::set_file_mtime( + &oldest_file, + filetime::FileTime::from_system_time(now - Duration::from_secs(180)), + ) + .expect("oldest mtime"); + + let service = CleanupService::new( + CleanupRoots { + temp_dir, + logs_dir, + cache_dir, + }, + CleanupPolicy { + max_cache_size_mb: 4, + ..CleanupPolicy::default() + }, + ); + + let result = service.cleanup_all().await.expect("cleanup"); + + assert_eq!(result.files_deleted, 1); + assert_eq!(result.bytes_freed, two_mb.len() as u64); + assert!(newest_file.exists()); + assert!(middle_file.exists()); + assert!(!oldest_file.exists()); + assert_eq!(result.categories.len(), 1); + assert_eq!(result.categories[0].name, "Oversized Cache"); +} + +#[tokio::test] +async fn token_usage_service_persists_records_and_filters_subagents_by_default() { + let temp = tempfile::tempdir().expect("tempdir"); + let service = + bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) + .await + .expect("service"); + + service + .record_usage( + "model-config-a".to_string(), + "model-a".to_string(), + "session-a".to_string(), + "turn-a".to_string(), + 100, + 40, + Some(30), + Some(json!({ "cacheCreationTokenCount": 12 })), + false, + ) + .await + .expect("record main"); + service + .record_usage( + "model-config-a".to_string(), + "model-a".to_string(), + "session-a".to_string(), + "turn-sub".to_string(), + 50, + 10, + None, + None, + true, + ) + .await + .expect("record subagent"); + + let summary = service + .get_summary(bitfun_services_core::token_usage::TokenUsageQuery { + model_id: Some("model-a".to_string()), + session_id: None, + time_range: bitfun_services_core::token_usage::TimeRange::All, + limit: None, + offset: None, + include_subagent: false, + }) + .await + .expect("summary"); + + assert_eq!(summary.record_count, 1); + assert_eq!(summary.total_input, 100); + assert_eq!(summary.total_cached, 30); + assert_eq!(summary.total_cache_write, 12); + + let reloaded = + bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) + .await + .expect("reloaded"); + let stats = reloaded + .get_model_stats("model-a") + .await + .expect("model stats"); + assert_eq!(stats.request_count, 2); + assert_eq!(stats.total_input, 150); +} + +#[tokio::test] +async fn token_usage_clear_does_not_replay_cached_record_batches() { + let temp = tempfile::tempdir().expect("tempdir"); + let service = + bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) + .await + .expect("service"); + + service + .record_usage( + "model-config-old".to_string(), + "model-old".to_string(), + "session-old".to_string(), + "turn-old".to_string(), + 10, + 5, + None, + None, + false, + ) + .await + .expect("record old usage"); + service.clear_all_stats().await.expect("clear usage"); + service + .record_usage( + "model-config-new".to_string(), + "model-new".to_string(), + "session-new".to_string(), + "turn-new".to_string(), + 20, + 7, + None, + None, + false, + ) + .await + .expect("record new usage"); + + let summary = service + .get_summary(bitfun_services_core::token_usage::TokenUsageQuery { + model_id: None, + session_id: None, + time_range: bitfun_services_core::token_usage::TimeRange::All, + limit: None, + offset: None, + include_subagent: true, + }) + .await + .expect("summary after clear"); + + assert_eq!(summary.record_count, 1); + assert_eq!(summary.total_input, 20); + assert!(service.get_model_stats("model-old").await.is_none()); + assert_eq!( + service + .get_model_stats("model-new") + .await + .expect("new model stats") + .request_count, + 1 + ); +} + +#[tokio::test] +async fn token_usage_all_range_ignores_non_date_record_files() { + let temp = tempfile::tempdir().expect("tempdir"); + let service = + bitfun_services_core::token_usage::TokenUsageService::new(temp.path().to_path_buf()) + .await + .expect("service"); + + service + .record_usage( + "model-config-a".to_string(), + "model-a".to_string(), + "session-a".to_string(), + "turn-a".to_string(), + 10, + 1, + None, + None, + false, + ) + .await + .expect("record usage"); + + let records_dir = temp.path().join("records"); + fs::write( + records_dir.join("manual-backup.json"), + r#"{"records":[{"model_id":"model-b","session_id":"session-b","turn_id":"turn-b","timestamp":"2026-07-07T00:00:00Z","input_tokens":999,"output_tokens":1,"cached_tokens":0,"cached_tokens_available":false,"cache_write_tokens":0,"total_tokens":1000,"token_details":null,"is_subagent":false}]}"#, + ) + .expect("write stray record file"); + + let summary = service + .get_summary(bitfun_services_core::token_usage::TokenUsageQuery { + model_id: None, + session_id: None, + time_range: bitfun_services_core::token_usage::TimeRange::All, + limit: None, + offset: None, + include_subagent: true, + }) + .await + .expect("summary"); + + assert_eq!(summary.record_count, 1); + assert_eq!(summary.total_input, 10); +} diff --git a/src/crates/services/services-integrations/AGENTS.md b/src/crates/services/services-integrations/AGENTS.md index 62065f349..3e6895503 100644 --- a/src/crates/services/services-integrations/AGENTS.md +++ b/src/crates/services/services-integrations/AGENTS.md @@ -96,13 +96,19 @@ slices that are outside pure product logic but still platform-neutral. ## Verification +Select one integration family and its minimum feature set. Remote SSH uses a +grouped target for tests within the same boundary; use +`--test ::` for a single source module instead of +creating another Cargo target. Real transport/system boundaries such as MCP +streamable HTTP stay independent. Representative stable entry points are: + ```bash -cargo test -p bitfun-services-integrations -cargo test -p bitfun-services-integrations --no-default-features --features plugin-source plugin_source --lib -cargo test -p bitfun-services-integrations --features debug-log --test debug_log_owner_contracts -cargo test -p bitfun-services-integrations --features remote-ssh --test remote_ssh_disabled_contracts -cargo test -p bitfun-services-integrations --features remote-ssh,workspace-search --test remote_workspace_search_disabled_contracts -cargo test -p bitfun-services-integrations --features remote-ssh,remote-ssh-concrete,workspace-search remote_ssh -node scripts/check-core-boundaries.mjs -cargo check -p bitfun-core --features product-full +cargo check -p bitfun-services-integrations --no-default-features +cargo test -p bitfun-services-integrations --no-default-features --features mcp --test mcp_contracts +cargo test -p bitfun-services-integrations --no-default-features --features remote-ssh --test remote_ssh_contracts remote_ssh_disabled_contracts:: +cargo test -p bitfun-services-integrations --no-default-features --features file-watch --test file_watch_contracts +pnpm run check:core-boundaries ``` + +Other family-specific targets remain in `Cargo.toml`; add a guide command only +for a recurring workflow, not to mirror every test target. diff --git a/src/crates/services/services-integrations/Cargo.toml b/src/crates/services/services-integrations/Cargo.toml index a8f027e4f..eb5ee30b4 100644 --- a/src/crates/services/services-integrations/Cargo.toml +++ b/src/crates/services/services-integrations/Cargo.toml @@ -4,6 +4,7 @@ version.workspace = true authors.workspace = true edition.workspace = true description = "BitFun integration service owner crate" +autotests = false [lib] name = "bitfun_services_integrations" @@ -406,54 +407,62 @@ tokio = { workspace = true, features = ["io-util", "macros", "net", "rt", "test- [[test]] name = "debug_log_owner_contracts" +path = "tests/debug_log_owner_contracts.rs" required-features = ["debug-log"] [[test]] name = "script_tool_runtime" +path = "tests/script_tool_runtime.rs" required-features = ["script-tool-runtime"] [[test]] name = "announcement_contracts" +path = "tests/announcement_contracts.rs" required-features = ["announcement"] [[test]] name = "file_watch_contracts" +path = "tests/file_watch_contracts.rs" required-features = ["file-watch"] [[test]] name = "function_agent_contracts" +path = "tests/function_agent_contracts.rs" required-features = ["function-agents"] [[test]] name = "git_contracts" +path = "tests/git_contracts.rs" required-features = ["git"] [[test]] name = "mcp_contracts" +path = "tests/mcp_contracts.rs" required-features = ["mcp"] [[test]] name = "mcp_streamable_http_contracts" +path = "tests/mcp_streamable_http_contracts.rs" required-features = ["mcp"] [[test]] name = "remote_connect_contracts" +path = "tests/remote_connect_contracts.rs" required-features = ["remote-connect"] [[test]] name = "remote_ssh_contracts" -required-features = ["remote-ssh"] - -[[test]] -name = "remote_ssh_disabled_contracts" +path = "tests/remote_ssh_contracts.rs" required-features = ["remote-ssh"] [[test]] name = "remote_workspace_search_disabled_contracts" +path = "tests/remote_workspace_search_disabled_contracts.rs" required-features = ["remote-ssh", "workspace-search"] [[test]] name = "workspace_search_contracts" +path = "tests/workspace_search_contracts.rs" required-features = ["workspace-search"] [lints] diff --git a/src/crates/services/services-integrations/tests/remote_ssh_contracts.rs b/src/crates/services/services-integrations/tests/remote_ssh_contracts.rs index 4dae3e309..4171c8ab1 100644 --- a/src/crates/services/services-integrations/tests/remote_ssh_contracts.rs +++ b/src/crates/services/services-integrations/tests/remote_ssh_contracts.rs @@ -1,348 +1,6 @@ #![cfg(feature = "remote-ssh")] -use bitfun_services_integrations::remote_ssh::{ - canonicalize_local_workspace_root, local_workspace_roots_equal, - local_workspace_stable_storage_id, normalize_local_workspace_root_for_stable_id, - normalize_remote_workspace_path, remote_root_to_mirror_subpath, remote_workspace_runtime_root, - remote_workspace_session_mirror_dir, remote_workspace_stable_id, - sanitize_remote_mirror_path_component, sanitize_ssh_connection_id_for_local_dir, - sanitize_ssh_hostname_for_mirror, unresolved_remote_session_storage_dir, - unresolved_remote_session_storage_key, workspace_logical_key, workspace_session_identity, - ContainerAccess, ContainerWorkspaceConfig, RemoteWorkspace, RemoteWorkspaceRegistry, - SSHAuthMethod, SSHConnectionConfig, SavedAuthType, SavedConnection, LOCAL_WORKSPACE_SSH_HOST, -}; - -#[test] -fn remote_ssh_legacy_agent_auth_keeps_default_private_key_fallback() { - let config: SSHConnectionConfig = serde_json::from_value(serde_json::json!({ - "id": "conn-1", - "name": "dev", - "host": "example.com", - "port": 22, - "username": "alice", - "auth": { "type": "Agent" }, - "defaultWorkspace": "/repo" - })) - .unwrap(); - - match config.auth { - SSHAuthMethod::Agent { - key_fingerprint, - fallback_key_path, - } => { - assert_eq!(key_fingerprint, None); - assert_eq!(fallback_key_path.as_deref(), Some("~/.ssh/id_rsa")); - } - _ => panic!("legacy agent auth must remain agent-compatible"), - } - assert_eq!(config.proxy_jump, None); - assert_eq!(config.container, None); - assert_eq!(config.options.connect_timeout_secs, 30); - assert_eq!(config.options.auth_timeout_secs, 60); - assert_eq!(config.options.auth_attempts, 3); - assert_eq!(config.options.connect_attempts, 1); - - let saved: SavedConnection = serde_json::from_value(serde_json::json!({ - "id": "conn-1", - "name": "dev", - "host": "example.com", - "port": 22, - "username": "alice", - "authType": { "type": "Agent" }, - "defaultWorkspace": "/repo", - "lastConnected": 1 - })) - .unwrap(); - - assert!(matches!( - saved.auth_type, - SavedAuthType::Agent { - key_fingerprint: None, - ref fallback_key_path, - } if fallback_key_path.as_deref() == Some("~/.ssh/id_rsa") - )); - assert_eq!(saved.proxy_jump, None); - assert_eq!(saved.container, None); - assert_eq!(saved.options.connect_timeout_secs, 30); - assert_eq!(saved.options.auth_timeout_secs, 60); - assert_eq!(saved.options.auth_attempts, 3); - assert_eq!(saved.options.connect_attempts, 1); -} - -#[test] -fn remote_target_contract_uses_proxy_jump_and_kebab_case_container_access() { - let config = SSHConnectionConfig { - id: "conn-1".to_string(), - name: "train".to_string(), - host: "train.internal".to_string(), - port: 22, - username: "trainer".to_string(), - auth: SSHAuthMethod::PrivateKey { - key_path: "~/.ssh/train".to_string(), - passphrase: None, - certificate_path: None, - }, - default_workspace: Some("/workspace".to_string()), - proxy_jump: Some("jump1,jump2".to_string()), - container: Some(ContainerWorkspaceConfig { - name: "trainer-dev".to_string(), - access: ContainerAccess::DockerExec, - local: false, - docker_path: "docker".to_string(), - shell: "/bin/bash".to_string(), - user: Some("trainer".to_string()), - interactive: true, - }), - options: Default::default(), - }; - - let json = serde_json::to_value(&config).unwrap(); - assert_eq!(json["proxyJump"], "jump1,jump2"); - assert_eq!(json["container"]["access"], "docker-exec"); - assert_eq!(json["container"]["dockerPath"], "docker"); - let round_trip: SSHConnectionConfig = serde_json::from_value(json).unwrap(); - assert_eq!( - round_trip.container.unwrap().access, - ContainerAccess::DockerExec - ); -} - -#[test] -fn remote_workspace_defaults_keep_older_files_loadable() { - let workspace: RemoteWorkspace = serde_json::from_value(serde_json::json!({ - "connectionId": "conn-1" - })) - .unwrap(); - - assert_eq!(workspace.connection_id, "conn-1"); - assert_eq!(workspace.remote_path, ""); - assert_eq!(workspace.connection_name, ""); - assert_eq!(workspace.ssh_host, ""); -} - -#[test] -fn remote_workspace_path_helpers_preserve_current_identity_contract() { - assert_eq!( - normalize_remote_workspace_path(r"\\home\\user\\repo//src"), - "/home/user/repo/src" - ); - assert_eq!(normalize_remote_workspace_path("///"), "/"); - assert_eq!( - normalize_remote_workspace_path("/home/user/repo/"), - "/home/user/repo" - ); - - #[cfg(windows)] - assert_eq!( - sanitize_ssh_connection_id_for_local_dir("ssh-root@1.95.50.146:22"), - "ssh-root@1.95.50.146-22" - ); - #[cfg(not(windows))] - assert_eq!( - sanitize_ssh_connection_id_for_local_dir("ssh-root@1.95.50.146:22"), - "ssh-root@1.95.50.146:22" - ); - assert_eq!( - sanitize_ssh_connection_id_for_local_dir("../unsafe/id"), - "..-unsafe-id" - ); - assert_eq!(sanitize_ssh_connection_id_for_local_dir(".."), "_dotdot_"); - - assert_eq!(sanitize_remote_mirror_path_component(""), "_"); - assert_eq!(sanitize_remote_mirror_path_component("."), "_dot_"); - assert_eq!(sanitize_remote_mirror_path_component(".."), "_dotdot_"); - assert!(remote_root_to_mirror_subpath("/../../escape") - .components() - .all(|component| !matches!(component, std::path::Component::ParentDir))); - assert_eq!( - remote_root_to_mirror_subpath("/home/user/../project"), - std::path::PathBuf::from("home").join("project"), - "safe legacy dot segments must keep their previous effective mirror path" - ); - #[cfg(windows)] - { - assert_eq!(sanitize_remote_mirror_path_component("CON"), "_CON"); - assert_eq!(sanitize_remote_mirror_path_component("report. "), "report"); - } - assert_eq!( - sanitize_ssh_hostname_for_mirror(" Example.COM "), - "example.com" - ); - assert_eq!( - remote_root_to_mirror_subpath("/home/user/repo"), - std::path::PathBuf::from("home").join("user").join("repo") - ); - assert_eq!( - remote_root_to_mirror_subpath("/"), - std::path::PathBuf::from("_root") - ); - - assert_eq!( - workspace_logical_key(LOCAL_WORKSPACE_SSH_HOST, "/Users/p/w"), - "localhost:/Users/p/w" - ); - - let local_id = local_workspace_stable_storage_id("/Users/foo/BitFun"); - assert_eq!(local_id, "local_1d9bbee7a88cb84fc9500423130a3e99"); - - let remote_id = remote_workspace_stable_id("myhost", "/root/proj"); - assert_eq!(remote_id, "remote_0b6e9c54b3e51fd56bf721ed35c1ce88"); - - let unresolved_key = unresolved_remote_session_storage_key(" conn-1 ", "/home/u/p"); - assert_eq!(unresolved_key, "d1c72f60fc1b7cb99599cf21"); -} - -#[test] -fn remote_workspace_session_paths_use_supplied_mirror_root() { - let mirror_root = std::path::PathBuf::from("/bitfun/remote_ssh"); - - assert_eq!( - remote_workspace_runtime_root(&mirror_root, " Example.COM ", "/home/user/repo"), - mirror_root - .join("example.com") - .join("home") - .join("user") - .join("repo") - ); - assert_eq!( - remote_workspace_session_mirror_dir(&mirror_root, " Example.COM ", "/"), - mirror_root - .join("example.com") - .join("_root") - .join("sessions") - ); - assert_eq!( - unresolved_remote_session_storage_dir(&mirror_root, " conn-1 ", "/home/u/p"), - mirror_root - .join("_unresolved") - .join("d1c72f60fc1b7cb99599cf21") - .join("sessions") - ); -} - -#[test] -fn local_workspace_identity_helpers_preserve_canonical_root_contract() { - let workspace_root = std::env::temp_dir().join(format!( - "bitfun-services-remote-ssh-contract-{}", - std::process::id() - )); - let nested = workspace_root.join("nested"); - std::fs::create_dir_all(&nested).expect("workspace root should exist"); - - let (canonical_path, stable_root) = - canonicalize_local_workspace_root(&workspace_root).expect("canonical local root"); - assert_eq!( - stable_root, - normalize_local_workspace_root_for_stable_id(&workspace_root) - .expect("normalized local root") - ); - assert_eq!( - stable_root, - canonical_path.to_string_lossy().replace('\\', "/") - ); - assert!(local_workspace_roots_equal( - &workspace_root, - &workspace_root - )); - assert!(!local_workspace_roots_equal(&workspace_root, &nested)); - - let _ = std::fs::remove_dir_all(workspace_root); -} - -#[test] -fn workspace_session_identity_preserves_local_and_remote_contracts() { - let workspace_root = std::env::temp_dir().join(format!( - "bitfun-services-workspace-identity-{}", - std::process::id() - )); - std::fs::create_dir_all(&workspace_root).expect("workspace root should exist"); - - let local = - workspace_session_identity(&workspace_root.to_string_lossy(), None, None).expect("local"); - assert_eq!(local.hostname, LOCAL_WORKSPACE_SSH_HOST); - assert!(!local.is_remote()); - assert_eq!(local.remote_connection_id, None); - - let remote = workspace_session_identity( - r"\\home\\wsp\\project//", - Some(" conn-1 "), - Some(" ssh.dev "), - ) - .expect("remote"); - assert_eq!(remote.hostname, "ssh.dev"); - assert_eq!(remote.logical_workspace_path(), "/home/wsp/project"); - assert_eq!(remote.remote_connection_id.as_deref(), Some("conn-1")); - assert!(remote.is_remote()); - - assert!( - workspace_session_identity("/home/wsp/project", Some("conn-1"), None).is_none(), - "remote identity requires a resolvable SSH host" - ); - - let _ = std::fs::remove_dir_all(workspace_root); -} - -#[tokio::test] -async fn remote_workspace_registry_preserves_ambiguous_root_resolution_contract() { - let registry = RemoteWorkspaceRegistry::new(); - registry - .register_remote_workspace( - "/".to_string(), - "conn-a".to_string(), - "Server A".to_string(), - "host-a".to_string(), - ) - .await; - registry - .register_remote_workspace( - "/".to_string(), - "conn-b".to_string(), - "Server B".to_string(), - "host-b".to_string(), - ) - .await; - - assert!(registry.lookup_connection("/tmp", None).await.is_none()); - - registry - .set_active_connection_hint(Some("conn-a".to_string())) - .await; - let hinted = registry.lookup_connection("/tmp", None).await.unwrap(); - assert_eq!(hinted.connection_id, "conn-a"); - assert_eq!(hinted.ssh_host, "host-a"); - - let preferred = registry - .lookup_connection("/tmp", Some("conn-b")) - .await - .unwrap(); - assert_eq!(preferred.connection_id, "conn-b"); - assert_eq!(preferred.ssh_host, "host-b"); -} - -#[tokio::test] -async fn remote_workspace_registry_preserves_legacy_state_and_clear_contract() { - let registry = RemoteWorkspaceRegistry::new(); - assert!(!registry.has_any().await); - assert!(!registry.get_state().await.is_active); - - registry - .register_remote_workspace( - "/repo".to_string(), - "conn-1".to_string(), - "Dev Server".to_string(), - "dev.example.com".to_string(), - ) - .await; - - let state = registry.get_state().await; - assert!(state.is_active); - assert_eq!(state.connection_id.as_deref(), Some("conn-1")); - assert_eq!(state.remote_path.as_deref(), Some("/repo")); - assert_eq!(state.connection_name.as_deref(), Some("Dev Server")); - - registry - .unregister_remote_workspace("conn-1", "/repo") - .await; - assert!(!registry.has_any().await); - assert!(!registry.get_state().await.is_active); -} +#[path = "remote_ssh_contracts/remote_ssh_contracts.rs"] +mod remote_ssh_contracts; +#[path = "remote_ssh_contracts/remote_ssh_disabled_contracts.rs"] +mod remote_ssh_disabled_contracts; diff --git a/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_contracts.rs b/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_contracts.rs new file mode 100644 index 000000000..4dae3e309 --- /dev/null +++ b/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_contracts.rs @@ -0,0 +1,348 @@ +#![cfg(feature = "remote-ssh")] + +use bitfun_services_integrations::remote_ssh::{ + canonicalize_local_workspace_root, local_workspace_roots_equal, + local_workspace_stable_storage_id, normalize_local_workspace_root_for_stable_id, + normalize_remote_workspace_path, remote_root_to_mirror_subpath, remote_workspace_runtime_root, + remote_workspace_session_mirror_dir, remote_workspace_stable_id, + sanitize_remote_mirror_path_component, sanitize_ssh_connection_id_for_local_dir, + sanitize_ssh_hostname_for_mirror, unresolved_remote_session_storage_dir, + unresolved_remote_session_storage_key, workspace_logical_key, workspace_session_identity, + ContainerAccess, ContainerWorkspaceConfig, RemoteWorkspace, RemoteWorkspaceRegistry, + SSHAuthMethod, SSHConnectionConfig, SavedAuthType, SavedConnection, LOCAL_WORKSPACE_SSH_HOST, +}; + +#[test] +fn remote_ssh_legacy_agent_auth_keeps_default_private_key_fallback() { + let config: SSHConnectionConfig = serde_json::from_value(serde_json::json!({ + "id": "conn-1", + "name": "dev", + "host": "example.com", + "port": 22, + "username": "alice", + "auth": { "type": "Agent" }, + "defaultWorkspace": "/repo" + })) + .unwrap(); + + match config.auth { + SSHAuthMethod::Agent { + key_fingerprint, + fallback_key_path, + } => { + assert_eq!(key_fingerprint, None); + assert_eq!(fallback_key_path.as_deref(), Some("~/.ssh/id_rsa")); + } + _ => panic!("legacy agent auth must remain agent-compatible"), + } + assert_eq!(config.proxy_jump, None); + assert_eq!(config.container, None); + assert_eq!(config.options.connect_timeout_secs, 30); + assert_eq!(config.options.auth_timeout_secs, 60); + assert_eq!(config.options.auth_attempts, 3); + assert_eq!(config.options.connect_attempts, 1); + + let saved: SavedConnection = serde_json::from_value(serde_json::json!({ + "id": "conn-1", + "name": "dev", + "host": "example.com", + "port": 22, + "username": "alice", + "authType": { "type": "Agent" }, + "defaultWorkspace": "/repo", + "lastConnected": 1 + })) + .unwrap(); + + assert!(matches!( + saved.auth_type, + SavedAuthType::Agent { + key_fingerprint: None, + ref fallback_key_path, + } if fallback_key_path.as_deref() == Some("~/.ssh/id_rsa") + )); + assert_eq!(saved.proxy_jump, None); + assert_eq!(saved.container, None); + assert_eq!(saved.options.connect_timeout_secs, 30); + assert_eq!(saved.options.auth_timeout_secs, 60); + assert_eq!(saved.options.auth_attempts, 3); + assert_eq!(saved.options.connect_attempts, 1); +} + +#[test] +fn remote_target_contract_uses_proxy_jump_and_kebab_case_container_access() { + let config = SSHConnectionConfig { + id: "conn-1".to_string(), + name: "train".to_string(), + host: "train.internal".to_string(), + port: 22, + username: "trainer".to_string(), + auth: SSHAuthMethod::PrivateKey { + key_path: "~/.ssh/train".to_string(), + passphrase: None, + certificate_path: None, + }, + default_workspace: Some("/workspace".to_string()), + proxy_jump: Some("jump1,jump2".to_string()), + container: Some(ContainerWorkspaceConfig { + name: "trainer-dev".to_string(), + access: ContainerAccess::DockerExec, + local: false, + docker_path: "docker".to_string(), + shell: "/bin/bash".to_string(), + user: Some("trainer".to_string()), + interactive: true, + }), + options: Default::default(), + }; + + let json = serde_json::to_value(&config).unwrap(); + assert_eq!(json["proxyJump"], "jump1,jump2"); + assert_eq!(json["container"]["access"], "docker-exec"); + assert_eq!(json["container"]["dockerPath"], "docker"); + let round_trip: SSHConnectionConfig = serde_json::from_value(json).unwrap(); + assert_eq!( + round_trip.container.unwrap().access, + ContainerAccess::DockerExec + ); +} + +#[test] +fn remote_workspace_defaults_keep_older_files_loadable() { + let workspace: RemoteWorkspace = serde_json::from_value(serde_json::json!({ + "connectionId": "conn-1" + })) + .unwrap(); + + assert_eq!(workspace.connection_id, "conn-1"); + assert_eq!(workspace.remote_path, ""); + assert_eq!(workspace.connection_name, ""); + assert_eq!(workspace.ssh_host, ""); +} + +#[test] +fn remote_workspace_path_helpers_preserve_current_identity_contract() { + assert_eq!( + normalize_remote_workspace_path(r"\\home\\user\\repo//src"), + "/home/user/repo/src" + ); + assert_eq!(normalize_remote_workspace_path("///"), "/"); + assert_eq!( + normalize_remote_workspace_path("/home/user/repo/"), + "/home/user/repo" + ); + + #[cfg(windows)] + assert_eq!( + sanitize_ssh_connection_id_for_local_dir("ssh-root@1.95.50.146:22"), + "ssh-root@1.95.50.146-22" + ); + #[cfg(not(windows))] + assert_eq!( + sanitize_ssh_connection_id_for_local_dir("ssh-root@1.95.50.146:22"), + "ssh-root@1.95.50.146:22" + ); + assert_eq!( + sanitize_ssh_connection_id_for_local_dir("../unsafe/id"), + "..-unsafe-id" + ); + assert_eq!(sanitize_ssh_connection_id_for_local_dir(".."), "_dotdot_"); + + assert_eq!(sanitize_remote_mirror_path_component(""), "_"); + assert_eq!(sanitize_remote_mirror_path_component("."), "_dot_"); + assert_eq!(sanitize_remote_mirror_path_component(".."), "_dotdot_"); + assert!(remote_root_to_mirror_subpath("/../../escape") + .components() + .all(|component| !matches!(component, std::path::Component::ParentDir))); + assert_eq!( + remote_root_to_mirror_subpath("/home/user/../project"), + std::path::PathBuf::from("home").join("project"), + "safe legacy dot segments must keep their previous effective mirror path" + ); + #[cfg(windows)] + { + assert_eq!(sanitize_remote_mirror_path_component("CON"), "_CON"); + assert_eq!(sanitize_remote_mirror_path_component("report. "), "report"); + } + assert_eq!( + sanitize_ssh_hostname_for_mirror(" Example.COM "), + "example.com" + ); + assert_eq!( + remote_root_to_mirror_subpath("/home/user/repo"), + std::path::PathBuf::from("home").join("user").join("repo") + ); + assert_eq!( + remote_root_to_mirror_subpath("/"), + std::path::PathBuf::from("_root") + ); + + assert_eq!( + workspace_logical_key(LOCAL_WORKSPACE_SSH_HOST, "/Users/p/w"), + "localhost:/Users/p/w" + ); + + let local_id = local_workspace_stable_storage_id("/Users/foo/BitFun"); + assert_eq!(local_id, "local_1d9bbee7a88cb84fc9500423130a3e99"); + + let remote_id = remote_workspace_stable_id("myhost", "/root/proj"); + assert_eq!(remote_id, "remote_0b6e9c54b3e51fd56bf721ed35c1ce88"); + + let unresolved_key = unresolved_remote_session_storage_key(" conn-1 ", "/home/u/p"); + assert_eq!(unresolved_key, "d1c72f60fc1b7cb99599cf21"); +} + +#[test] +fn remote_workspace_session_paths_use_supplied_mirror_root() { + let mirror_root = std::path::PathBuf::from("/bitfun/remote_ssh"); + + assert_eq!( + remote_workspace_runtime_root(&mirror_root, " Example.COM ", "/home/user/repo"), + mirror_root + .join("example.com") + .join("home") + .join("user") + .join("repo") + ); + assert_eq!( + remote_workspace_session_mirror_dir(&mirror_root, " Example.COM ", "/"), + mirror_root + .join("example.com") + .join("_root") + .join("sessions") + ); + assert_eq!( + unresolved_remote_session_storage_dir(&mirror_root, " conn-1 ", "/home/u/p"), + mirror_root + .join("_unresolved") + .join("d1c72f60fc1b7cb99599cf21") + .join("sessions") + ); +} + +#[test] +fn local_workspace_identity_helpers_preserve_canonical_root_contract() { + let workspace_root = std::env::temp_dir().join(format!( + "bitfun-services-remote-ssh-contract-{}", + std::process::id() + )); + let nested = workspace_root.join("nested"); + std::fs::create_dir_all(&nested).expect("workspace root should exist"); + + let (canonical_path, stable_root) = + canonicalize_local_workspace_root(&workspace_root).expect("canonical local root"); + assert_eq!( + stable_root, + normalize_local_workspace_root_for_stable_id(&workspace_root) + .expect("normalized local root") + ); + assert_eq!( + stable_root, + canonical_path.to_string_lossy().replace('\\', "/") + ); + assert!(local_workspace_roots_equal( + &workspace_root, + &workspace_root + )); + assert!(!local_workspace_roots_equal(&workspace_root, &nested)); + + let _ = std::fs::remove_dir_all(workspace_root); +} + +#[test] +fn workspace_session_identity_preserves_local_and_remote_contracts() { + let workspace_root = std::env::temp_dir().join(format!( + "bitfun-services-workspace-identity-{}", + std::process::id() + )); + std::fs::create_dir_all(&workspace_root).expect("workspace root should exist"); + + let local = + workspace_session_identity(&workspace_root.to_string_lossy(), None, None).expect("local"); + assert_eq!(local.hostname, LOCAL_WORKSPACE_SSH_HOST); + assert!(!local.is_remote()); + assert_eq!(local.remote_connection_id, None); + + let remote = workspace_session_identity( + r"\\home\\wsp\\project//", + Some(" conn-1 "), + Some(" ssh.dev "), + ) + .expect("remote"); + assert_eq!(remote.hostname, "ssh.dev"); + assert_eq!(remote.logical_workspace_path(), "/home/wsp/project"); + assert_eq!(remote.remote_connection_id.as_deref(), Some("conn-1")); + assert!(remote.is_remote()); + + assert!( + workspace_session_identity("/home/wsp/project", Some("conn-1"), None).is_none(), + "remote identity requires a resolvable SSH host" + ); + + let _ = std::fs::remove_dir_all(workspace_root); +} + +#[tokio::test] +async fn remote_workspace_registry_preserves_ambiguous_root_resolution_contract() { + let registry = RemoteWorkspaceRegistry::new(); + registry + .register_remote_workspace( + "/".to_string(), + "conn-a".to_string(), + "Server A".to_string(), + "host-a".to_string(), + ) + .await; + registry + .register_remote_workspace( + "/".to_string(), + "conn-b".to_string(), + "Server B".to_string(), + "host-b".to_string(), + ) + .await; + + assert!(registry.lookup_connection("/tmp", None).await.is_none()); + + registry + .set_active_connection_hint(Some("conn-a".to_string())) + .await; + let hinted = registry.lookup_connection("/tmp", None).await.unwrap(); + assert_eq!(hinted.connection_id, "conn-a"); + assert_eq!(hinted.ssh_host, "host-a"); + + let preferred = registry + .lookup_connection("/tmp", Some("conn-b")) + .await + .unwrap(); + assert_eq!(preferred.connection_id, "conn-b"); + assert_eq!(preferred.ssh_host, "host-b"); +} + +#[tokio::test] +async fn remote_workspace_registry_preserves_legacy_state_and_clear_contract() { + let registry = RemoteWorkspaceRegistry::new(); + assert!(!registry.has_any().await); + assert!(!registry.get_state().await.is_active); + + registry + .register_remote_workspace( + "/repo".to_string(), + "conn-1".to_string(), + "Dev Server".to_string(), + "dev.example.com".to_string(), + ) + .await; + + let state = registry.get_state().await; + assert!(state.is_active); + assert_eq!(state.connection_id.as_deref(), Some("conn-1")); + assert_eq!(state.remote_path.as_deref(), Some("/repo")); + assert_eq!(state.connection_name.as_deref(), Some("Dev Server")); + + registry + .unregister_remote_workspace("conn-1", "/repo") + .await; + assert!(!registry.has_any().await); + assert!(!registry.get_state().await.is_active); +} diff --git a/src/crates/services/services-integrations/tests/remote_ssh_disabled_contracts.rs b/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs similarity index 100% rename from src/crates/services/services-integrations/tests/remote_ssh_disabled_contracts.rs rename to src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs