PicoBot/docs/CONFIG_HOT_RELOAD_REVIEW.md

26 KiB
Raw Permalink Blame History

配置热重载功能审核报告

状态:审查完成;主要意见已于 2026-07-21 落地。本文前六节保留首次审查快照,行号、测试数和“当前实现”描述可能已过时;以下处置表与代码为最新结论。

本文是对 docs/CONFIG_HOT_RELOAD_DESIGN.md 描述的配置热重载功能及其当前未提交实现的综合审核。审核覆盖架构合理性、实现一致性、关键不变量和改进建议;具体行为以代码和测试为最终依据。相关实现位于 src/gateway/reload.rssrc/gateway/mod.rs::runsrc/config/mod.rs::load_for_reloadsrc/session/session.rs::wait_until_idlesrc/tools/reload_config.rssrc/gateway/http.rs::reload_config

0. Review 意见处置与实现结果

意见 处置 结论与实现
A1 / E1 MCP 准备期副作用 接收 connect_all() 移到运行代激活阶段,候选构造不再启动双份 MCP 或提前覆盖全局 status。
A2 / E2 generation 与状态查询 接收 每次请求分配 generation新增 GET /api/config/reload/status,状态为 preparing/draining/activating/active/failed
A3 激活失败回滚 部分接收 接收“需要明确失败状态”,但驳回先启动新 Channel 再停旧 Channel。飞书长连接双开会重复消费事件风险高于短暂切换空窗完整 prepare/activate 或可恢复旧代留作后续。
A4 / L4 Agent 工具风险 部分接收 保留用户要求的 reload_config,但只允许根交互 Agent 使用;子 Agent、Cron、managed scheduled Agent 均剔除该工具,并保持 exclusive。驳回直接删除工具。
A5 / E5 后台任务排空 接收 引入运行代 admission/activity guardScheduler 已执行 job 与后台子 Agent 持有 guard新任务在 drain 后不再进入。
I1 / E4 Slash 回复可能丢失 接收 /reload 所在 inbound 在 lane 入队前持有 guardcommand output 改为显式等待 outbound delivery acknowledgement删除固定 500ms sleep。
I2 / F2 DB 路径误判 接收 比较归一化后的有效路径,None./picobot.db 可判为同一数据库。
I3 零散预校验 部分接收 保留 default agent 与 Feishu 的快速错误提示;其余组件统一由候选 from_config() 构造验证,不继续扩张 ad-hoc 字段校验。长期采用显式 prepare contract。
I4 HTTP 错误码 接收 pending 返回 409退出/准备故障返回 503配置与不可变字段错误返回 400。错误改为 ReloadError 类型。
I5 候选双 Storage 驳回为正确性缺陷 同库多连接是 sqlx/SQLite 的正常模式,候选无消息入口;路径身份仍被强制保持一致。该点保留为测试与锁竞争观察项,而非阻止上线。
F1 集成测试 部分接收 新增真实 Gateway 子进程测试,覆盖成功切换到 generation 2、状态查询、无效候选不影响旧代健康。可控假 Provider、长 Turn 和故障注入仍待补充。
L1 prepare/activate 分离 方向接收 本轮已把 MCP activation 与候选构造分离;完整组件级接口留作后续架构演进。
L2 动态 Router 暂缓 当前保留 listener 并为 Axum graceful shutdown 增加 10 秒硬上限;不为本功能引入动态 service 复杂度。
L3 客户端完成通知 暂缓 后端 generation/status 已具备TUI/WebUI 展示可在后续独立实现。

首次 Review 未指出、但本轮一并修复的两个关键问题:候选构造原先直接在 select! 分支内 await会暂停轮询旧 Axum serve 与进程信号;现在 serve 独立受监督运行,准备和排空阶段都继续响应服务退出。其次,原实现排空前没有关闭入口,持续新消息可能让排空永不稳定;现在 admission 先关闭再 drain。

1. 审查范围与依据

1.1 审查对象

  • 设计文档:docs/CONFIG_HOT_RELOAD_DESIGN.md
  • 实现:当前未提交的 17 个文件变更与 2 个新增文件(src/gateway/reload.rssrc/tools/reload_config.rs),共 ~304 行净增
  • 关联变更:Cargo.tomlwebui/package.json 版本号 1.2.2 → 1.3.0README.mdAGENTS.mddocs/ARCHITECTURE.mdresources/skills/about-picobot/references/config.md 文档同步

1.2 审查依据

  • 仓库既有架构边界与并发不变量(见 docs/ARCHITECTURE.md
  • 现有相似机制Channel 生命周期、TaskSupervisor、TurnController、OutboundDispatcher
  • 验证命令:cargo buildcargo clippy --all-targets --all-features -- -D warningscargo test --lib330 passedwebui && npm run check && npm run build

1.3 验证结果

命令 结果
cargo build 通过
cargo clippy --all-targets --all-features -- -D warnings 通过
cargo test --lib 330 passed / 0 failed
cd webui && npm run check && npm run build 0 errors / 0 warnings

2. 架构审查

2.1 整体架构评估

结论架构方向正确运行代runtime generation切换模型是 PicoBot 配置散落现状下的唯一可靠方案。

PicoBot Gateway 启动时把配置拆分复制到 SessionManagerChannelManager、MCP、Scheduler、Browser、Auth、Upload 等长生命周期组件;gateway.host/port、进程 cwd、SQLite 连接具有进程级生命周期。在这种结构下,任何"原地替换 Config 指针"的方案都会导致请求处理组件混用新旧配置(新会话用新模型、旧 Session 仍用旧 Provider或飞书配置显示禁用但旧连接仍在接收消息

运行代切换通过"先在旧代仍服务时构造完整候选代;候选可用后排空当前交互工作,再回收旧代并激活新代"避免了半套配置暴露。这一选择与 PicoBot 单进程、单 Gateway 模型契合,不引入 daemon/fork 层。

2.2 运行代模型合理性

运行代模型的关键设计点均合理:

设计点 评估
保留原始 std::net::TcpListener,每代 try_clone() 避免 bind 失败与端口抢占,保留内核 backlog
候选构造期间不修改旧 GatewayState 避免半套配置暴露
Config::load_for_reload 使用启动环境快照、不调 set_var 多线程运行期修改进程环境的危险被正确规避
不可变字段host/port/workspace/db_path边界清晰 边界划分正确
三个入口CLI、/reloadreload_config 工具)共享控制通道 单一校验路径,语义一致
拒绝 nginx 式新旧 worker 长期并行 单进程规模不值得这份复杂度
ReloadHandle 用有界 MPSC + try_send 无界积压被正确拒绝

2.3 边界划分评估

热重载边界表(设计文档第 8 节)覆盖完整:

  • 可热重载:providers/models/agentschannelsmcpbrowsermemorygateway.schedulergateway.max_concurrent_background_tasksgateway.file_transfergateway.require_pairing
  • 必须重启:gateway.host/portworkspace_dirgateway.session_db_path、未进入配置组件的继承环境变量

边界划分与实现中 load_candidate() 的校验项一一对应。workspace_dir 在比较前按启动 cwd 解析并 canonicalizereload.rs:80-89),与 from_configensure_workspace_dir 的 canonicalize 行为一致,比较基准正确。

2.4 并发不变量评估

设计文档第 10 节列出的 10 条不变量在实现中均得到遵守:

不变量 实现位置 遵守情况
只有 run() 拥有 receiver 与当前运行代 mod.rs:331 ReloadController 在 run() 内创建
候选构造不修改旧 GatewayState mod.rs:399 from_config 创建独立 state
不在热重载路径调 env::set_var config/mod.rs:611 apply_to_process=false
不释放原始 listener 后 rebind mod.rs:345 listener 在 run() 内持有
旧 WebSocket 观察 connection_shutdown mod.rs:421 切换前 cancel
旧任务由旧 TaskSupervisor 回收 mod.rs:432-436 旧 supervisor shutdown
排空检查不长时间持有 Session mutex session.rs:2113-2124 先克隆 Arc 再短锁
reload tool 保持 exclusive reload_config.rs:49 exclusive: true
不可变字段比较在候选构造之前 reload.rs:80-100
新增启动期配置消费者需更新边界表 文档约束 ⚠️ 维护性约束,无机制强制

3. 实现审查

3.1 与设计文档的一致性

实现与设计文档的关键路径高度一致:

设计文档章节 实现位置 一致性
§6.1 准备阶段load_candidate → from_config mod.rs:386-408
§6.2 接受响应:候选构造成功后通过 oneshot 返回 mod.rs:410-411
§6.3 排空阶段wait_until_idle 60s + 500ms 投递窗口 mod.rs:412-419
§6.4 切换顺序connection_shutdown → generation_shutdown → channel stop → task_supervisor → 新代启动 mod.rs:421-422, 429-436, 350-351
§4 控制通道:容量 8、try_send、队列满立即返回错误 reload.rs:8, 47-54
§7.2 环境变量语义:不修改进程环境 config/mod.rs:541-553
§9 失败语义:候选构造失败时旧代不变 mod.rs:394-408 continue 不切换

3.2 关键路径分析

3.2.1 切换时序

gateway::run 的主循环(mod.rs:349-441)正确实现了运行代切换:

外层 loop {
  start_all / start_message_processing  // 新代激活
  内层 loop { select! { serve | process_signal | reload_request } }
  serve.await                           // 等待旧 axum graceful shutdown
  channel_manager.stop_all              // 停止旧 channel intake
  task_supervisor.cancel + shutdown(10s)// 回收旧受监督任务
  state = next_state                    // 切换
}

wait_for_shutdown_signal() 在外层循环内创建(mod.rs:365),每代新建 future不存在重复 poll 已完成 future 的 UB。

3.2.2 候选构造与旧代隔离

from_configmod.rs:51-236)为候选创建独立的 Storage、MessageBus、ChannelManager、SessionManager、ToolRegistry、AuthManager、UploadRegistry。候选的 channel_manager.init() 仅构造 Channel 对象,不调用 start(),因此不会与旧 channel 并发连接飞书 API。

3.2.3 监听 socket 保留

std::net::TcpListenermod.rs:345)在进程生命周期内持有;每代通过 try_clone()mod.rs:353)获得新 fd。旧代 serve future 退出时仅释放克隆 fd原始 socket 不释放,新代可重新克隆并 accept。内核 backlog 在切换窗口中暂存新 TCP 连接。

3.3 测试覆盖评估

结论:单元测试覆盖不足,集成测试完全缺失。

当前测试:

测试 位置 覆盖范围
candidate_accepts_runtime_changes_and_rejects_workspace_changes reload.rs:127 load_candidate 的 model_id 变更接受与 workspace 拒绝
resolve_slash_command("reload") session.rs:3485 slash 命令解析

缺失但设计文档第 12 节明确列出的测试:

  1. 启动真实 Gateway、修改 model/channel 配置、HTTP 触发重载、验证新代生效
  2. 配置无效时验证旧 WebSocket 与旧 Provider 仍可工作
  3. 长 Turn 中调用 reload_config、验证 tool result 与最终消息投递后才断开
  4. Session 队列积压时验证重载等待排空
  5. 排空超时、Channel stop 超时、候选激活失败的故障注入
  6. 重载前后鉴权策略变化与旧 WebSocket 失效

当前测试仅覆盖纯函数路径(load_candidate、slash 解析),未验证任何运行时切换行为。这是上线前的主要风险点。

3.4 代码质量

  • Clippy-D warnings 通过
  • 类型安全ReloadHandle::unavailable()reload.rs:39)通过 drop receiver 使 try_send 返回 Closed,正确表达"Gateway 未由 run() 启动"的语义
  • 错误处理:候选构造失败时 request.response.send(Err(...))continue不切换MCP 单 server 失败沿用启动语义(记录错误、跳过工具),不阻断候选构造
  • 资源管理:旧 TaskSupervisor 的 10s 有界 shutdown + abort 保证回收有硬时间边界

4. 发现的问题

4.1 架构层面问题

A1. MCP 在准备阶段连接,制造进程级副作用【中】

from_configmod.rs:171)调用 mcp::connect_all(),立即建立 MCP 客户端连接或启动 stdio 子进程,并更新进程级 MCP_SERVER_STATUSmcp/mod.rs:42)。这违反了设计文档第 10 节不变量 #2"候选构造不得修改旧 GatewayState"的精神——MCP status 虽非请求处理状态,但仍是旧代可见的进程级状态。

具体影响:

  • 候选构造期间,旧代的 /mcp 命令显示候选的连接状态而非旧代状态
  • stdio MCP 子进程双份运行(旧代 + 候选)可能竞争资源或 stdin/stdout
  • 候选在 connect_all 之后失败(如 ensure_default_maintenance_job 失败,mod.rs:194MCP 连接被 drop 但 MCP_SERVER_STATUS 仍显示 connected: true,旧代 /mcp 显示陈旧数据

设计文档第 6.1 节与第 14 节将此列为"已知例外"。但该例外的收益仅为"提前发现 MCP 失败"——而 MCP 单 server 失败本就被当非致命跳过(mcp/mod.rs:171),不需要提前连接来验证。

A2. 无 generation ID 与状态查询【中】

调用方只能得到准备阶段结果("配置校验通过Gateway 将在当前任务结束后切换到新配置"),无法查询重载最终是否完成。运维需要翻日志或重连客户端确认切换状态。设计文档第 14 节将此列为"演进方向",但 generation ID一个 atomic 计数器)+ /api/config/reload/status 端点成本极低,收益显著,应在 v1 内置。

A3. 候选激活失败无回滚【中-高】

切换顺序为:停旧 channel → 取消旧 TaskSupervisor → 切换 state → 启动新 channelmod.rs:429-351)。若新代 start_all() 失败,run() 返回错误,依赖 systemd 拉起。但 channel 启动失败常为瞬时问题(飞书 5xx、端口冲突丢掉本来正常服务的旧代去重启是可用性损失。旧代此时已被回收无法回滚。

A4. reload_config Agent 工具的软约束【低-中】

ReloadConfigToolreload_config.rs)注册到默认工具集,依赖 description"仅在用户明确要求重新加载配置时调用"约束 LLM。LLM compliance 是软约束,非可靠边界。exclusive: true 仅保证不与其他副作用工具并行,不保证调用时机正确。

A5. 排空仅覆盖交互 Session不覆盖 Scheduler/后台 Agent【低】

wait_until_idle 仅检查内存中 Session 的 current_cancelagent_tx 队列。Scheduler job、独立后台子 Agent、HTTP handler 不在排空范围内。Scheduler job 可能正在写 DB 或发送消息,被 TaskSupervisor 10s abort 截断可能留下不一致状态。设计文档第 6.3 节已承认此范围。

4.2 实现层面问题

I1. /reload slash command 绕过 wait_until_idle【中】

session.rs:2631 显示 slash command 在 handle_message 内联处理,返回 HandleResult::CommandOutput不进入 session worker 队列。因此 wait_until_idle 检查 current_cancel/agent_tx.capacity() 时看不到 /reload Turn 的活动状态,立即返回(仅 100ms 稳定 + 500ms sleep

slash command output 经 process_inboundpublish_command_output → outbound dispatcher → channel API 投递,整条链路必须在 ~600ms+ serve graceful shutdown + 10s TaskSupervisor shutdown内完成。对 CLI channel 足够,但对 Feishu 等远端 channel 较紧。设计文档第 6.3 节"为 slash command output 和终态投递留出发送窗口"承认了 500ms 窗口,但 500ms 是固定值,无背压保证。

实际窗口因 TaskSupervisor 的 10s shutdown 较宽,不会丢消息——但若 channel stop_all() 关闭了连接in-flight 的 send_message 可能失败。

I2. session_db_path 比较为原始字符串【低】

reload.rs:91 直接比较 current.gateway.session_db_path != candidate.gateway.session_db_path。若用户把 null 改为 "./picobot.db"(解析后同一路径),会被误拒。保守是对的,但产生假阳性。workspace_dir 已做 canonicalize 比较,session_db_path 应保持一致。

I3. load_candidate 仅校验 Feishu 凭据【低】

reload.rs:73-78 仅校验飞书 app_id/app_secret 非空。其他 channel 配置若有、MCP 配置、Browser 配置等在 from_config 期间才验证,可能在那里失败。这与设计文档第 6.1 节一致("若飞书启用,校验 app_id 和 app_secret 非空"),但将失败发现延后到了候选构造阶段。

I4. reload_config HTTP handler 错误码语义【低】

http.rs:355 对所有错误用 ApiError::bad_request400。"gateway is shutting down"(队列关闭)更适合 503 Service Unavailable"another configuration reload is already pending"更适合 409 Conflict。

I5. 候选 Storage 与旧 Storage 共享同一 SQLite 文件【低】

from_config 为候选创建新 Storage 连接到同一 picobot.db。候选的 background notification consumer 与 cleanup task 已归候选 TaskSupervisorcleanup 跳过首次 tick无入口触发 notification故实际不写。理论上有并发写锁竞争可能实际风险低。设计文档未显式说明此点。

4.3 严重度分级

问题 严重度 影响
A3 候选激活失败无回滚 中-高 瞬时 channel 故障导致整个 Gateway 重启
A1 MCP 准备阶段连接 进程级 status 污染、子进程双份、失败后状态陈旧
A2 无 generation ID 运维无法确认切换完成状态
I1 /reload 绕过排空 远端 channel output 投递窗口紧
A4 reload_config 软约束 低-中 LLM 误调用风险
A5 排空不覆盖 Scheduler 后台 job 被硬取消可能留下不一致
I2 session_db_path 假阳性 用户需重启而非重载
I3 仅校验 Feishu 失败发现延后
I4 HTTP 错误码 语义不准确
I5 双 Storage 连接 理论并发风险

5. 改进建议

5.1 必须修复(上线前)

F1. 补充端到端集成测试

至少覆盖设计文档第 12 节列出的前 3 项:

  1. 启动真实 Gateway、修改 model_id、HTTP 触发重载、验证新 model 在新 Turn 中生效
  2. 配置无效(如 default agent 解析失败)时验证旧 WebSocket 与旧 Provider 仍可工作
  3. 长 Turn 中调用 reload_config、验证 tool result 与最终消息投递后才断开

这些测试无法用单元测试替代,需启动真实 Gateway 进程。

F2. session_db_path 比较归一化

reload.rs:91 应将 session_db_path 相对于 workspace 解析并 canonicalize 后比较,与 workspace_dir 处理方式一致。None"picobot.db"(默认值)应视为等价。

5.2 建议增强(近期演进)

E1. MCP 连接移出 from_config,消除"已知例外"

mcp::connect_all()from_configmod.rs:171)移到 start_all() 阶段,与 channel 启动同相位。收益:

  • 消除进程级 MCP_SERVER_STATUS 在候选构造期间被污染
  • 消除 stdio 子进程双份运行
  • 候选失败时无 MCP 连接泄漏
  • 不再需要设计文档第 10 节不变量 #2 的"已知例外"声明

代价MCP 连接失败从"候选构造失败"延后到"激活失败"。但 MCP 单 server 失败本就非致命(跳过该 server 工具),整体激活失败语义不变。

E2. 引入 generation ID 与 status 查询

ReloadController 增加 Arc<AtomicU64> generation 计数器与 ReloadState enumIdle/Preparing/Draining/Activating/Active/Failed)。提供 GET /api/config/reload/status 端点。调用方在收到 "accepted" 后可轮询确认切换完成。成本极低(一个 atomic + 一个路由 + 一个 enum运维收益显著。

E3. 旧代保留至新代 start_all() 成功

调整切换顺序为:先启动新 channel新代 channel_manager.start_all成功后再停止旧 channel。短暂双 channel 并存对飞书 webhook 幂等消息可容忍。代价是需处理两代 channel 并存的资源冲突(如媒体目录、飞书事件去重),但避免瞬时 channel 故障导致 Gateway 整体重启。

若实现成本过高,至少应在 start_all() 失败时尝试重启旧代 channel旧 TaskSupervisor 已 cancel可能无法恢复需评估可行性

E4. /reload slash command 排空路径

两种方案:

  • 方案 A:让 reload controller 记录触发源channel, chat_id等待该 inbound lane 的当前消息处理完成后再切换,而非泛化等待所有 session idle
  • 方案 B:为 outbound dispatcher 增加显式 drain contractstop_all 前等待 outbound 队列排空或超时

方案 A 更精确,方案 B 更通用。两者都比固定 500ms sleep 可靠。

E5. Scheduler 与后台 job 协作式 drain

在 Scheduler job 的协作取消边界检查 reload token允许 job 在写 DB 前/后选择继续完成或退出。避免 TaskSupervisor 10s abort 截断 DB 写一半的 job。设计文档第 14 节已列出此项。

5.3 演进方向(中长期)

L1. prepare() / activate() 显式分离

from_config 拆为:

  • construct():纯内存,无 I/O可反复调用
  • prepare():可失败的 I/Ochannel health check、MCP 连接、Storage ping旧代仍服务
  • activate():开始接收消息

使更多失败前移到旧代仍可回退的阶段,而非等到 activate 才暴露。设计文档第 14 节已列出此项。

L2. 动态 Router service

引入新旧 HTTP generation 短期重叠,消除 accept/Channel intake 空窗。需不破坏 Channel/Session 边界。设计文档第 14 节已列出此项。

L3. WebUI/TUI reload 完成通知

客户端在 WebSocket 重连后显示 reload 完成状态与自动重连提示。依赖 E2 的 generation ID。

L4. 重新评估 reload_config Agent 工具

考虑移除该工具,仅保留 CLI 与 /reload。Agent 触发进程级状态切换的风险A4可能不抵边际收益。若保留应在 SessionManager 层做调用方校验(如要求参数带确认 token而非靠 description。

6. 总体结论

6.1 设计评估

设计文档质量高,边界清晰,不变量明确,失败语义完整。运行代切换模型是 PicoBot 当前架构下的正确选择。设计文档诚实地列出了已知限制(第 14 节),未掩饰缺陷。

主要设计层面的不足是把几件本应 v1 内置的能力generation ID、MCP 相位对齐、旧代保留至新代激活成功)推迟到"演进方向"导致可用性与可观测性打了折扣。MCP 的"已知例外"A1是设计妥协被文档化的典型留着会让后续维护者也认为"再来一个例外无所谓",应尽早消除而非长期承担。

6.2 实现评估

实现忠实遵循设计,关键不变量均得到遵守。代码通过 cargo buildcargo clippy -D warningscargo test --lib330 passed与 WebUI check/build。版本号、文档、AGENTS.md 同步更新。

主要实现层面的不足是测试覆盖:仅 load_candidate 与 slash 解析有单元测试无任何运行时切换行为的集成验证I1-I5 中多数问题需要集成测试才能暴露)。设计文档第 12 节明确列出但未实现的 6 项集成测试是上线前的主要风险。

6.3 上线建议

判定
架构方向 可接受
实现一致性 可接受
代码质量 可接受Clippy/tests/build 全通过)
测试覆盖 ⚠️ 不足需补集成测试F1
已知限制 ⚠️ 文档已承认,但 A1/A3 应优先修复

建议:在完成 F1集成测试与 F2session_db_path 归一化后可提交。A1MCP 相位、A3无回滚应列为后续优先修复项不应长期承担。

附录:关键文件与符号索引

文件/符号 职责 行号
src/gateway/reload.rs::ReloadController 重载控制通道、候选配置加载、不可变字段校验 19-36
src/gateway/reload.rs::ReloadHandle 可克隆的请求端,注入 SessionManager/工具/HTTP state 14-59
src/gateway/reload.rs::load_candidate 候选配置解析与不可变字段校验 61-102
src/gateway/mod.rs::run 持有 listener、当前运行代与切换主循环 318-441
src/gateway/mod.rs::GatewayState::from_config 构造一套完整运行代依赖 51-236
src/gateway/mod.rs::build_router 为每代构建 Axum Router 448-489
src/config/mod.rs::Config::load_for_reload 使用启动环境/cwd 安全重新解析配置 541-553
src/config/mod.rs::Config::load_from_with_process_env 共享的配置加载实现,支持不写入进程环境 547-625
src/session/session.rs::wait_until_idle 检查活动 Turn、Session 队列与稳定空闲窗口 2109-2144
src/session/session.rs::execute_slash_command /reload slash 入口 2093-2099
src/tools/reload_config.rs::ReloadConfigTool Agent 可调用的独占重载工具 6-52
src/gateway/http.rs::reload_config 受保护的 POST /api/config/reload 347-360
src/client/mod.rs::reload_gateway CLI HTTP 客户端与 bearer token 注入 101-124
src/main.rs::Command::Reload picobot reload CLI 定义 59-64, 132-138
src/mcp/mod.rs::connect_all MCP 连接(当前在 from_config 期间调用,见 A1 126-181
src/mcp/mod.rs::MCP_SERVER_STATUS 进程级 MCP 状态A1 的副作用源) 36-45