feat(eval): 完成统一评测平台与 GAIA 官方适配器 - #305
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
已同步 CodeWhale PR #15 的评审修复并更新 gitlink 到 父仓 GAIA 受限轮次不再安装 shell-backed 本地 |
d5a0723 to
865323a
Compare
asto18089
left a comment
asto18089
left a comment
There was a problem hiding this comment.
评审结论:Request changes
感谢这个庞大而认真的 PR。平台的骨架质量很高——私有预测的 opaque handle + 四元绑定 + DPAPI/Unix 权限复检、fetch/verify 全链路 fail-closed(固定 revision/size/SHA-256、LFS 与 git blob 双轨校验、附件硬编码信任锚)、scorer 对官方 question_scorer 的逐项忠实移植、错误码不泄路径与内容,这些都做得扎实。但本次审阅(已将 PR rebase 到最新 main 并实跑全部测试)发现:PR 没有真正实现它声称的根因——GAIA 端到端评测在当前形态下跑不通,另有多处阻断级缺陷与范围污染。以下问题按严重度排列,均有文件:行号证据。
一、根因不成立(阻断)
1. GAIA 逐轮工具白名单与 v0.9.5 底座目录完全脱节,评测轮实际零工具
eval_tool_policy.rs 的两个 profile 使用 v0.9.3 之前的 per-action 旧名(read_file/list_dir/grep_files/file_search/web_search/fetch_url),而底座 @169c24cc 的注册面只有 canonical 家族名(File/Web,registry.rs:717-720、925-930;底座钉板测试 registry.rs:2453-2476 明确断言旧名不注册、不出现在模型目录)。链路后果:
- 目录过滤
tool_matches_any_rule(tool_catalog.rs:509-527)只做小写精确名/*前缀匹配,无别名→家族映射 → 模型目录被过滤到空(offline profile 全灭,public-web 仅剩条件注册的image_analyze); - 模型若调用 canonical
File/Web,exact 门禁(tool_execution.rs:390-398,ExactToolDispatchPolicy::allows为原始名逐字相等,ops.rs:60-62)直接拒绝; - 模型若伪造
read_file,底座未注册该执行体,fail-closed 到 "no execution path"。
两条路都死:GAIA 产品运行退化为无工具纯文本回答,带附件题目连附件都读不了。PR 自己的断言测试(eval_tool_policy.rs:151-195 断言 read_file 等在产品目录恰好出现一次)与底座钉板测试正面矛盾——即便编译通过也必败。需要按 canonical 家族名 + action 粒度重新设计白名单(可能涉及 CodeWhale PR #15 的 exact 门禁语义支持 action 级解析),这不是局部修补能完成的。
2. 评分闭环缺少官方语义:无 FINAL ANSWER 指令注入与提取
全仓 grep FINAL ANSWER 零命中。prompt 是 parquet 原始 Question(private_inputs.rs:42-45),持久化的 prediction 是最后一轮 assistant 全文拼接(headless_bridge.rs:882),scorer 拿全文与 ground truth 做归一化等值比较(scorer.rs:98)。设计文档(docs/superpowers/specs/2026-08-13-pinvou-benchmark-platform-design.md §13)自己要求了 FINAL ANSWER 提取与 missing_final_answer 处理,实现缺位。真实运行几乎逐题判错,submission 的 model_answer 是整段对话文本,不具备声称的官方可比性。OutputContract("gaia-final/v1") 是纯标签,无消费点。
二、编译/测试断裂(阻断,均已本地复现)
SessionStore::load_skill_bindings已被 main 的 #285 移除,PR 仍调用(headless_bridge.rs:991、eval_cli.rs:73)→benchmark-hooks与dev-toolsfeature 编译失败(E0599),pinvou-cli 默认构建(主链路)同样断裂。cargo test --lib三处编译错误,主 CI 门禁必挂:engine.rs:2652base_op()缺新 gitlink 的turn_tool_security字段(E0063);eval_tool_policy.rs:87import 已不存在的deepseek_tui::tools::pinvou3_blocklist(E0432);:92import 私有模块worker_profile(E0603)。forwarder.rs:834与:904每轮双重 finish:第一次finish_turn_with_usage已 pop 队首,第二次finish_turn_with_observation常规轮 no-op——PR 核心观测数据(tool_calls/catalog 摘要)永不落盘;排队双轮场景还会 pop 错下一轮,写出错配的假assistant_done,前端重读时间线即错乱(GUI 回归)。应删掉旧调用只留 observation 版(注意保持"先落时间线再发 chat:done"的顺序),并补排队双轮测试。submission.rs:202-233validate_parent逐级拒绝 symlink 祖先:macOS 的/var(temp_dir 祖先)是 symlink,导致$TMPDIR下写 submission 永远gaia_submission_target_unsafe,cargo test -p adapter-gaia在 macOS 两红(已实测复现)。真正防覆写靠create_new+hard_link,祖先检查收益低、代价是 macOS 不可用,建议只校验叶子父目录。tests/headless_bridge_contract.rs:276-285断言attachments_runtime_unsupported,但RecordingRuntime走 trait 默认实现 bailunsupported_tool_policy,错误映射(headless_bridge.rs:829-843)不覆盖该串 → 非 Windows 必挂。- GAIA 测试自身缺陷(已在本地修复验证):
contract_tests.rs/dataset_contract.rs的TempSnapshot用纳秒时间戳命名,macOS 粗粒度时钟下同 tick 撞名,并行测试互相remove_dir_all,约 1/3 概率随机红;fetch_contract.rs临时目录未设 0700,Unix 默认 umask 下必触发 fetch 的私有权限契约(ImportFailed)。修复各一行量级,建议作者采纳。
三、CI 门禁缺口(阻断——上述断裂全部因此漏网)
- 全仓没有任何 CI job 编译/测试
pinvou-cliworkspace(约 3 万行),benchmark-hooksfeature 后的headless_bridge.rs(1558 行)+headless_bridge_contract.rs(806 行)零编译零运行;adapter-gaia覆盖最重的dataset_contract还挂在required-features=["test-support"]下,裸cargo test会静默跳过。至少需要:pinvou-cli 的 fmt+clippy+cargo test --all-features门禁、app 侧--features benchmark-hooks的编译与契约测试门禁。 verify-public-submodule.sh --allow-registered-candidate在pull_request与merge_group都放行未打不可变标签的 gitlink,而 fast-gate 的 job 级条件不含 push——严格标签校验在 CI 中已成死代码,main 的可复现性将依赖 CodeWhale 侧一个可变分支不被 force-push/GC。要么 merge_group 去掉 flag,要么在 fork-modifications.md 写明候选提升为不可变标签(r8)的后续责任与时限。另:fork-guard.sh:98-99两条新指纹指向的测试(restricted_turn_hooks_require_explicit_host_opt_in、restricted_tool_audit_redacts_private_sentinel)不带forkguard_前缀,完整 guard 的 layer-2 过滤器永远选不中它们——恰是"封闭 Hook/日志旁路"的核心回归,需上游改名或扩展过滤器。
四、底座(CodeWhale PR #15 候选,169c24cc)安全缺口
Op::ContinueGoal处理器不安装 per-op 策略(engine.rs:2146-2207vs SendMessage 的 :2099-2101):若 exact 白名单含update_goal,goal 自调度续轮将以完全无限制状态运行(MCP/子 agent/hook/未脱敏日志全部恢复)。当前 GAIA profile 不含update_goal,属潜在缺口,但底座边界设计不应依赖白名单内容巧合;建议受限态拒绝续轮或继承策略,并补三处续轮 gate 的 forkguard 测试。Op::ReloadMcp(engine.rs:2701)无control_plane_restricted检查,受限轮期间排队的 MCP reload 会真实拉起外部进程,与"排队控制操作继承受限"的声称不符。- 受限轮审计事件整体替换为
{"event":"restricted_tool_event","details":"redacted"}(tool_execution.rs:186),连非私有的 tool_name 都丢弃,受限轮无法审计"哪个工具跑了几次",建议保留工具名、脱敏锁定在 input/output/路径。
五、范围污染与重复建设
- 夹带需拆分或删除:根目录
PROGRESS.md(235 行个人开发 checkpoint,含本机用户名路径与内部分支名,性质同 AGENTS.md §0 的.codex-memory.md,不应入库);docs/superpowers/17 份 agent 工具过程文档(约 4900 行,main 无此目录惯例,含 sub-skill 指令与 checkbox 状态);pinvou-cli/target/.rustc_info.json与CACHEDIR.TAG两个构建产物(需删除并在.gitignore补pinvou-cli/target/)。 - 同一 PR 内两套 smoke 评测栈并存:
features/assistant/eval/(app 侧)与pinvou-cli/crates/adapter-smoke(CLI 侧)是同 5 条 case、rules、product_score、judge、markdown 报告的完整重写。设计文档 §5.1 明确要求迁移验证后删除旧栈、不得双栈演进,PR 却两套都留。请按设计做出取舍。 pinvou3-app/src-tauri/Cargo.lock全树刷新(~200 个包版本漂移、36 移除、22 新增)远超"新增 optional path 依赖"所需,请基于 main 的 lock 最小化再生或显式说明。- GAIA run 不落
report.md/score.json,pinvou benchmark report对 GAIA 恒返回report_not_available,与docs/gaia-benchmark.md:88的声称矛盾——闭环最后一环缺失。
六、应改(不阻断但需回应)
- benchmark-core:JSONL 追加为两次写、读取不容忍残缺行,崩溃在最需要恢复的场景下让 run 永久不可 resume(
store.rs:440-460);单任务私有预测持久化失败中止整个 run(service.rs:244);plainresume()把 durable run 静默降级为 ephemeral(service.rs:123-137);record_outcome16KB 行上限 vs 无界 tool_observations。 - fetch 获取锁(
fetch.rs:2026-2039)无陈旧恢复,崩溃后永久卡死且错误码误导为gaia_verify_failed;下载路径附件信任 HF 自报 digest,未用代码里已硬编码的 11 条官方 digest 交叉校验,与"固定 revision/digest"主张有落差。 run_eval_smoke无 feature 门控注册进生产 invoke handler(lib.rs:923),单次 IPC 即以 Yolo 全权限跑真实模型;delete_then_forget改变既有删除语义、engine_support.rs通知默认值翻转、GUI 会话 sidecar 新增 tool 记录——这些借本 PR 落地的行为变更应在 PR 说明中显式声明。- 评测会话(标题"临时评测")未从 GUI 会话列表过滤,崩溃残留的
eval_*会话会把私密题目带进用户历史(sessions/store.rs:189只过滤sched-)。 - CLI 手写
json_escape+ 字符串拼 JSON(cli/src/lib.rs:803-809等),不转义控制字符,建议改用已是依赖的serde_json;cli_contract.rs的安全负向测试被cfg(not(feature = "product-backend"))门控,默认配置永不运行。 docs/gaia-native-turn-tool-policy.md状态仍标"拟实施"且候选名单含已被移除的retrieve_tool_result;docs/fork-modifications.md:171的表述与候选 gitlink 状态不符;scorer 固定 revision 在 platform-design §3.2(9f133d71…)与代码(1349a179…)之间不一致。run_with_staged_attachments(无策略变体)先把私有附件拷进 execution root 再无条件 bail,失败路径有副作用且是死代码(headless_bridge.rs:203-221)。- Windows 上 GAIA 带附件任务全部 fail-closed 不可跑,发布说明应点名此限制。
总结
这件事对产品有真实意义(模型/底座回归与诚实自评平台),隐私工程与 fetch/verify 链路的质量高于一般水平;但按当前交付形态价值未兑现:核心链路(工具面、评分语义)不成立、CI 看不见新代码、范围混入过程产物。建议按上文分组拆分处理:先修编译与 CI 门禁(二、三),再重做工具白名单与 FINAL ANSWER 闭环(一),范围清理(五)独立提交。底座侧缺口(四)请随 CodeWhale PR #15 一并处理。修改面较大,评审方不代改;如需逐项讨论的原始证据链可继续回复本评论。
asto18089
left a comment
asto18089
left a comment
There was a problem hiding this comment.
评审更新(针对新版 02efcece,已 rebase 到最新 main 9da6b87c 重验)
作者新推的 3 个修复提交(865323ac/b29f54bd/02efcece)方向正确,上一轮最致命的问题已解。我逐条复核了上轮评论的 17 项,结论:5 项已修复、1 项部分修复、10 项未动,另有 1 个本轮新发现的 macOS 阻断缺陷,以及 1 处我上轮的错误指控需要更正。维持 request changes。
一、已修复(逐条验证过)
- GAIA 工具白名单命名(原评论一.1,阻断)——已修复。两个 profile 改用 canonical 家族名
File/Web/image_analyze(eval_tool_policy.rs:4-6)。我沿运行时链路验证到底:底座 agent 注册面注册的工具字面名就是File(registry.rs:717-720,FileTool::new("File"))、Web(:925-930)与image_analyze(vision/tools.rs:158);ExactToolDispatchPolicy::allows(ops.rs:60-62)逐字匹配 → 目录过滤与 exact 门禁两条路现在都通。新增的test_headless_boot_contract.py家族名契约测试会被 CI 的unittest discover(pr-check.yml:316)自动收集,接线正确。 load_skill_bindings退役调用(原二.3)——已修复。两处调用点删除,并有 boot 契约测试兜底。cargo test --lib三处编译错误(原二.4)——已修复。turn_tool_security: None、pinvou3_blocklist/worker_profile两个坏 import 都已处理;cargo check --lib --tests实测零 error。- Cargo.lock 全树漂移(原五.16)——已修复。新版 lock 相对 main 仅 +12/-2(新增
agent-backend-api),是最小化再生。 - 排队控制操作旁路(原四.11 的大部分)——已修复。底座新提交
169c24cc给RunShellCommand/SpawnSubAgent加了control_plane_restricted锁存拒绝(engine.rs:2223/2264),并补了forkguard_queued_control_op_keeps_restricted_turn_authority回归测试,文档与指纹同步到位。 - Windows ACL 解析改为字节拷贝校验(
865323ac),消除了原始指针 ACE 遍历,顺带答了 CodeQL。
二、部分修复,残留需作者决策
- 工具粒度:家族名修复恢复了功能,但底座 agent 注册面的
File是全量读写FileTool(registry.rs:1155→with_file_tools()),GAIA offline/public-web 白名单放行的File因此包含 write/delete/move 等写动作,与 PR 自己声明的只读意图(原 per-action 名单只有 read/list/grep/search)不符。精确门禁目前只支持整工具名,action 级收窄需要底座支持(可随 CodeWhale PR #15 一并设计)。请明确决策:接受家族级粒度并在文档中声明,或等 action 级能力。 - forkguard 指纹前缀(原三.10 后半):新增的两条 T2 指纹已带
forkguard_前缀,但我指出的restricted_turn_hooks_require_explicit_host_opt_in、restricted_tool_audit_redacts_private_sentinel(fork-guard.sh:98-99)仍无前缀,完整 guard 的 layer-2 过滤器依然选不中这两条核心回归。
三、仍未修复(与上轮证据一致,均已在新版复核)
- FINAL ANSWER 闭环缺失(原一.2,阻断):全仓仍零命中;prediction 仍是末轮全文拼接,
OutputContract("gaia-final/v1")仍是无消费点的标签,scorer 仍拿全文做等值比较。官方可比性仍不成立。 - forwarder 双重 finish(原二.5,阻断):
forwarder.rs:834/904原样未动,观测数据(tool_calls/catalog 摘要)仍永不落盘,排队双轮仍会 pop 错。 validate_parent拒绝 symlink 祖先(原二.6):submission.rs:202-233未动,macOS$TMPDIR下写 submission 恒失败的问题仍在。headless_bridge_contract.rs:276-285断言(原二.7):RecordingRuntime仍未实现run_with_staged_attachments_and_policy,错误映射(headless_bridge.rs:825-840)仍不覆盖unsupported_tool_policy→ 非 Windows 必挂的逻辑未变。- GAIA 测试自身 flake(原二.8):
TempSnapshot纳秒命名并行撞名仍在(src/contract_tests.rs:34、tests/dataset_contract.rs:93,本轮实测并行 2 红、串行全绿);fetch_contract.rs临时目录 0700 加固未加(我在TMPDIR为 0755 权限目录的环境下实测 3 红)。修复各一行量级,建议采纳。 - CI 对 pinvou-cli 零覆盖(原三.1,阻断):仍无任何 workflow 引用
pinvou-cli;benchmark-hooks后的headless_bridge.rs与其契约测试仍零编译零运行。正因为此,下面第四节的新缺陷才能漏网。 --allow-registered-candidate在 merge_group 放行(原三.10 前半):pr-check.yml:294 未动,严格标签校验仍是死代码。- 范围污染(原五.14):
PROGRESS.md与docs/superpowers/(17 份过程文档)仍在树内。 - 双 smoke 栈(原五.15):
features/assistant/eval/与pinvou-cli/crates/adapter-smoke仍并存。 - GAIA 不落 report.md/score.json(原五.17):仍无实现,与
docs/gaia-benchmark.md:88矛盾仍在。 - 底座残留缺口(原四.11/12/13):
Op::ContinueGoal处理器仍不检查control_plane_restricted也不安装 per-op 策略(engine.rs:2146起);Op::ReloadMcp(:2701)仍无受限检查;受限审计仍整体 redacted 丢工具名(tool_execution.rs:189-197)。请随 CodeWhale PR #15 处理。 - 上轮第六部分(非阻断项)均未动,抽查确认:
run_eval_smoke仍无 feature 门控注册进生产 invoke handler(lib.rs:923);会话列表仍只过滤sched-(store.rs:189),eval_*残留会话仍会进用户历史;CLI 仍手写json_escape。
四、本轮新发现(macOS 阻断)
_EMBED_INFO_PLIST 重复符号,PR 主链路测试在 macOS 无法链接。 pinvou3-tauri 一个 crate 内现有三处 tauri::generate_context!():lib.rs:928(无条件,既有)+ eval_cli.rs:64(dev-tools,PR 新增)+ headless_bridge.rs:983(benchmark-hooks,PR 新增)。embed_plist 的 embed_info_plist_bytes! 宏以 #[no_mangle] static _EMBED_INFO_PLIST 故意让重复嵌入成为编译错误。实测(用 PR 自带 lockfile,tauri-build 2.6.1):cargo test -p pinvou-cli(默认 features 经 pinvou-product-backend 启用 benchmark-hooks → 两处宏展开)链接失败,0 个测试能跑;cargo test --test headless_bridge_contract --features benchmark-hooks 同样被堵。Linux/Windows 不受影响(embed_plist 是 macOS 专属依赖),所以 CI 是绿的——这正是缺 macOS/CLI 门禁的代价。建议把 headless/eval 入口改为复用 lib.rs 的单一 context 构造,或把 plist 嵌入收敛到最终 bin。
五、对我上轮评论的更正
上轮五.14 中"pinvou-cli/target/.rustc_info.json 与 CACHEDIR.TAG 两个构建产物入库"的说法是错的:我逐一核对了 PR 各版本(be0b06ac/46504fa9/d5a07234/02efcece)的 tree,pinvou-cli/target/ 从未被跟踪——那是我本地测试时不小心 git add -A 进自己验证分支的污染。向作者致歉,该子项作废(.gitignore 补 pinvou-cli/target/ 仍是值得做的防御性动作)。
总结
三个修复提交质量好、都打在点上,工具链路的根因已真正打通(粒度问题见二);但评分语义闭环(三.1)、观测落盘(三.2)、CI 门禁(三.6)这些阻断项未动,又新增一个 macOS 链接阻断(四),范围清理也未开始。修改面仍然较大,评审方继续不代改。我本地的 rebase+验证分支(基于 9da6b87c,含 lockfile 再生与一处 rebase 解法:main 已含 tempfile = "3",rebase 重放会产生重复键,需删 PR 侧那行)可随时推给作者参考。
评审修复总结（针对第二轮评审，
|
asto18089
left a comment
asto18089
left a comment
There was a problem hiding this comment.
审阅结论：Request Changes
感谢这份高质量的评测平台工作——隐私边界、digest 信任链和 fork 登记经逐项核验是真实可信的。但存在 2 个代码 BLOCKER（均已在本机复现）和多项 MAJOR，当前修订无法合并；合并还依赖 CodeWhale 侧流程收尾。以下按严重级列出，均附文件:行号。
审阅基础：已将 PR 净差异 rebase 到最新 origin/main（c65fa498，零内容冲突，GitHub MERGEABLE 属实），并在本地完成验证：pinvou-cli 22 套件 160 测试全绿、architecture-guard.py / fork-guard.sh --fast / cargo fmt 全绿、默认 feature 的 pinvou3-app cargo check --lib --tests 通过。标注【已复现】的条目为本机亲手验证，其余为深读代码所得。
BLOCKER
B1【已复现】benchmark run smoke 在真实产品后端上必然全部任务失败
pinvou-cli/crates/adapter-smoke/src/lib.rs:58与pinvou-cli/crates/cli/src/lib.rs:977硬编码工具策略pinvou-product/v1；- 但产品侧
pinvou3-app/src-tauri/src/features/assistant/product_runtime/eval_tool_policy.rs:61-69的resolve_eval_policy只注册了pinvou-gaia-public-web/v1与pinvou-gaia-offline/v1； - 链路：
PrepareRequest.tool_policy→headless_bridge.rs:683-689逐 id 解析 → 未注册即unsupported_tool_policy→runner.rs:175-176折叠为backend_prepare_failed。
结果：默认 product-backend feature 构建下，smoke 的每个任务 prepare 即失败，无法完成一次成功运行。CI 的两个测试档（--all-features / --no-default-features）都不经真实 headless 宿主执行 smoke，故门禁拦不住。修复方向：smoke 改用已注册的 GAIA 策略 id，或注册 pinvou-product/v1。
B2【已复现】benchmark-hooks feature 开启时，806 行 headless 契约测试编译失败
cargo test --manifest-path pinvou3-app/src-tauri/Cargo.toml \
--features benchmark-hooks --test headless_bridge_contract
报 E0407: method run_with_staged_attachments is not a member of trait ProductRuntimePort 与 E0599（AttachmentAwareRuntime 上无此方法）。该 feature 默认关闭、CI 不开，PR 声称的 pinvou-cli --all-features 验证覆盖不到这个面——即这份契约测试从未被编译通过过。trait 定义与 impl 需要同步修正，且建议把 cargo check --features benchmark-hooks 纳入门禁（见 M8）。
B3（流程阻塞，非代码缺陷）合并前置链未完成
pinvou-v0.9.5-r8 标签不存在、CodeWhale PR #15 未合并、scripts/verify-public-submodule.sh:7 的 PINVOU_CODEWHALE_TAG 仍为 r7。merge queue 严格模式会硬挡（fail-closed 有效，已验证脚本逻辑）。已知并需注意的约束：r8 必须以 fast-forward 方式精确指向 21e5f661——若 #15 经 GitHub merge/squash 产生新提交，则 gitlink 既非 r7 head 也非候选、commit 计数失配，fork-guard.sh:24-35 将在 main 上永久红灯，父仓两脚本 + fork-modifications.md 三处都要同步改。
MAJOR
评分语义与答案提取（直接压低准确率/可信度）
- M1 货币符号只剥
$ % ,，漏€ £：adapter-gaia/src/scorer.rs:114-115。GAIA 有大量欧元/英镑题，€1,250vs1250会被系统性判错。官方 normalize_number 同时剥€£。 - M2 FINAL ANSWER 空尾标记跨边界回退：
headless_bridge.rs:605-610用rsplit(MARKER).filter(!empty).next()——最后标记尾部为空时会取到更左侧的正文，如"…Paris\nFINAL ANSWER:"会把答题过程全文当答案送进 scorer，而非按契约报missing_final_answer。docs 声称「提取最后一个非空标记」，实现是「最后一个非空段」。 - M3 标记匹配过脆：
MARKER大小写敏感、冒号严格、不截断行尾。Final Answer: 42（常见 casing）、**FINAL ANSWER: 42**、标记后带解释文本等真实模型输出变体都会失败或污染答案。建议行边界 + 容错解析。 - M4 「官方评分」声明与证据不符：scorer 是未与官方 Python scorer 逐题交叉验证的 Rust 重实现（
docs/gaia-benchmark.md:105自己承认），存在可枚举分歧（M1、列表分支顺序敏感且元素不去标点、INFINITY哨兵 vs Python 有限哨兵）。但 golden 测试全部以自身实现为 oracle 并冠名official_*（tests/gaia_contract.rs:50-96）。既然 scorer revision 已 pin，请补一次真实 Python scorer 交叉验证，或把 PR 正文与命名降级为「自有重实现」。 - M5 一票否决的闭环设计：任何一题 Timeout/missing_final_answer → 整 run
unofficial_partial且禁止导出 submission（submission.rs:150-154、scorer.rs:103-107）。46+ 题、600s 上限的 web-agent 运行几乎必然有失败，PR 声称的「评分闭环」在真实运行中大概率终止于 partial。建议允许带缺失导出（官方口径按错误计）。
benchmark-core 持久化与恢复（与「恢复」卖点冲突）
- M6 并发 resume 同一 run 无互斥：
RunStore的锁是进程内 per-instanceArc<Mutex<()>>（store.rs:27），无 run 级 lockfile。两个进程（或同进程两次调用）resume 同一 run → 重复 outcome 行 →scorer.rs:69行数不等 → 静默partial(0,0)，无任何错误提示指向并发。 - M7 单行损坏砖死整个 run：
store.rs:467-480任何 JSONL 行解析失败即invalid_persisted_record，无 torn-tail（末行截断可丢弃）与中部损坏的区分，resume 永远失败。同类：create 在 mkdir 与 manifest 落盘之间失败会永久占用 run_id（store.rs:231-243）；schema_version写了但读侧从不校验（store.rs:31、event.rs:21）。
隐私与平台残留（整体是强项，仅两处缺口）
- M8 崩溃遗留 acquisition 锁永久阻塞 fetch：
fetch.rs:2026-2039锁文件仅由Drop移除，SIGKILL/断电后benchmark fetch gaia永久gaia_verify_failed（错误码还误导）。nonce 与文件身份已记录却未用于陈旧锁检测。 - M9 Windows 预测目录无 ACL 强制：数据集侧有完整 DACL 机制，预测 blob 目录在非 Unix 下
create_dir后既不设置也不校验 ACL（private_prediction.rs:434-437、497-503），且 envelope 明文integrity_digest放大为离线字典验证通道。
CI 覆盖与配置
- M10 ~19k 行新代码默认零门禁：pinvou-cli 六 crate +
headless_bridge.rs（1584 行）+ 契约测试只在ci:full-benchmark标签触发，默认 PR/merge queue/main 一律不编译不测试；11668 行新依赖不进任何 cargo-deny。8 条新增 forkguard 安全行为测试在 main 上从不执行。benchmark*四个 path-filter outputs 无消费者（死配置，制造「有分层覆盖」错觉）。建议至少把cargo check/test --manifest-path pinvou-cli/Cargo.toml（或 benchmark job 的 merge_group 分支）纳入 required gate——B2 正是这个缺口的直接后果。 - M11 CLI 无诊断通道：
cli/src/lib.rs多处把完整错误链折叠为smoke_run_failed/gaia_run_failed机器码且无--verbose/RUST_LOG逃生口，GAIA fetch 失败时无法区分网络/磁盘/token/校验错误。防泄漏与可诊断不是二选一，分级错误码即可。 - M12 工具观察白名单与策略工具名不一致：
benchmark-core/src/runner.rs:36-54只认产品内部名（web_search等），GAIA 策略允许的File/Web/image_analyze全被改写为[redacted-tool]——GAIA run 的工具观察全部失真，且底座 crate 反向硬编码上层工具目录属分层倒置。
行为变更声明
- M13 未声明的 GUI 行为变更应拆分：普通（非 scheduled）聊天会话的 token 总量持久化为新增行为——
engine.rs:1162-1170（spawn 捕获 base）、forwarder.rs:722-767（终态累加 + 失败改判Failed）、store.rs:571-597（persist_chat_token_total）。实现质量尚可，但这是面向全体 GUI 用户、与评测平台无依赖关系的变更，PR 正文只声明了另外三个。建议拆成独立 PR 单独评审与声明。 - 已声明的三个行为变更本身实现正确：
delete_then_forget不会产生数据孤儿（会话列表来自磁盘扫描、错误保留上抛、NotFound 幂等）；通知默认不发对 GUI 零影响（lib.rs无条件 manage NotificationState，已核实）；timing 老 sidecar 兼容路径完整。eval_前缀会话不进用户列表 +benchmark-hooksfeature 门控 + 显式None传参的三重隔离也验证成立。
正面确认（均已核验）
- fork 登记真实：gitlink
21e5f661经 gh api 全链溯源（CodeWhale PR #15 head、r7 公开基线之上纯前向 4 提交、13 commit 计数吻合）；8 条新 T2 指纹在该提交上全部存在；候选白名单双脚本硬编码 + 策略测试锁定，无静默绕过通道。 - 隐私边界经得起逐路径追踪：events/predictions/report/submission 四类公开产物只含 task_id、聚合计数与随机 handle，有 sentinel 级测试锁定；digest 信任链端到端（URL pin + 双哈希流式校验 + 发布后重验 + 目录白名单）；token 仅环境变量传入、无隐式回退、Debug 全脱敏。
- 卫生干净：82 文件全部可归入声称范围，无关混入≈0；无敏感信息/个人路径/过程文档残留；21 个提交 DCO 合规。所谓「退役 app 侧旧 smoke 栈」实为本 PR 开发过程中自建自删，main 上从未存在——建议把
features/assistant/eval/mod.rs:3-6的「已统一实现并删除」注释改为不误导评审者的表述。
合并前条件（建议）
- 修复 B1、B2（B2 修复后请实际跑通
--features benchmark-hooks契约测试）； - 完成评分正确性最小集：M1 + M2 + M3（M4/M5 至少把 PR 正文措辞与 docs 的保守表述对齐）；
- M6/M7 二选一：修复，或在 docs 明确标注当前单进程独占、无 torn-tail 容忍的已知限制；
- M13 拆分为独立 PR；
- M10 补最小门禁（
cargo check+ 契约测试进 merge queue）； - 走完 CodeWhale #15 合并 + r8 标签（精确指向
21e5f661）+ gitlink/TAG bump。
修改面较大，本轮只评审不代修改。总体而言这是一件意义真实、安全工程质量高于平均的研发基建，期待修复后合入。
|
已推送评审修复，当前 head：d9c89dc65。 本轮修复：
实际验证：
未在本机验证：
|
|
复评结论（基于 current head 阻塞问题
仍需处理
已确认
|
asto18089
left a comment
asto18089
left a comment
There was a problem hiding this comment.
第四轮复审结论：Request Changes（已 rebase 到最新 main 重验）
审阅基础：PR head d9c89dc65 已 rebase 到 origin/main（a7c23943，含 #300/#184/#275/#321/#324，冲突已全部解决）。rebase 后本地验证：pinvou-cli 22 套件 167 测试全绿、cargo check --lib --tests 通过、cargo fmt --check 干净、architecture-guard.py / fork-guard.sh --fast 全绿。
先说结论：平台的主体链路经逐项深审成立——三策略 id 注册、canonical 工具面 + exact 门禁 + 只读 File 投影的多重防线（底座侧未发现越权路径）、FINAL ANSWER 闭环、隐私产物的 sentinel 边界、fetch/verify 信任链、fork 登记拓扑。d9c89dc65 新增的执行锁/torn-tail 修复/duplicate outcome 拒绝/ProductV1 注册也都真实有效。
但 @h3c-hexin 今早提出的 3 个阻塞问题在本 head 中全部未修，其中阻塞 1 我在 rebase 后的树上实测复现。另有多项本轮新发现。以下按严重度列出，均附文件:行号（行号以 rebase 后树为准，与 PR head 可能有个位数偏移）。
一、h3c-hexin 三阻塞：全部未修复
B1【实测复现】benchmark-hooks 真实单测失败仍未修
rebase 后实跑：
cargo test --features benchmark-hooks --lib features::assistant::product_runtime::headless_bridge:: -- --test-threads=1
test ...::policy_defaults_fail_closed_without_calling_legacy_runtime ... FAILED
left: "attachments_runtime_unsupported"
right: "unsupported_tool_policy"
test result: FAILED. 9 passed; 1 failed
ProductRuntimePort::run_with_staged_attachments_and_policy 的 trait 默认实现 bail attachments_runtime_unsupported（headless_bridge.rs:85），测试在 :1140 断言 unsupported_tool_policy。d9c89dc65 只修了 Linux trait 签名编译（AttachmentAwareRuntime 统一到新方法），没有触碰这处断言错配。
更深层的问题：这条失败在任何 CI job 里都看不见——cargo test --lib --features benchmark-hooks 在全部 workflow 中从未执行（benchmark-contract 只 cargo check --test headless_bridge_contract，不编译 lib 的 cfg(test)；rust-test 是默认 features；benchmark-test 需 ci:full-benchmark 标签且也只跑 --test）。建议把它纳入 benchmark-contract（见 D5）。
修复是一行级：断言改为 attachments_runtime_unsupported，或默认实现统一到 unsupported_tool_policy——取决于作者想表达的语义（我倾向后者：对 legacy runtime 而言"不支持策略化运行"比"不支持附件"更准确）。
B2 score → resume → 再 score 的产物永久冲突：未修
score_gaia（cli/src/lib.rs:749-753）与open_gaia_run全程不取执行锁，也不检查 run 是否仍有未完成任务；部分完成的 run 可评分并写死score.json/report.md（partial）。- 之后 resume 补完再 score，产物字节必然不同 →
gaia_score_artifact_conflict（:832-837、:844-860），且无任何清除/覆盖途径——该 run 的评分工件永久无法再发布。 - 无该序列的回归测试（现有
gaia_score_publishes_machine_and_markdown_artifacts只覆盖一次性成功发布）。
建议三选一：score 与执行共享锁并拒绝可恢复 run；或允许基于 outcome digest 的安全替换；或至少提供 --force 重发布并补测试。
B3 plan 阶段崩溃后 resume 静默漏跑：未修
plan_tasks 仍逐任务追加 Planned 事件（store.rs:321-327，非原子）；中途崩溃后 JSONL 合法但任务集不完整（repair_torn_jsonl_tail 只修无换行的末行）。resume_adapter 重新生成完整 plan 但不补缺失的 Planned 事件（service.rs:163-165 不调 plan_tasks）；execute 只跑 recovered runnable 集（:212-216），最终可报 remaining=0 而部分任务从未执行。无回归测试。
二、评分正确性（直接影响"官方可比"主张）
M1【高，待交叉验证】与官方 scorer 的两处可翻转分歧
- gt 含逗号数字：官方
question_scorer对 ground_truth 走parse float(str.replace(",",""))（数字分支）；本实现parse_python_float不剥逗号（scorer.rs:119-129），gt"1,000"落入列表分支（split 成["1","000"]）→ 模型答"1000"被判错。normalize_number（模型侧）剥逗号而 gt 侧不剥，是不对称的。 - 列表比较有序 vs 无序：本实现
zip+ 长度相等（scorer.rs:36-48）；官方流传实现是set(...) == set(...)。乱序答案、重复项场景结论相反。现有 golden（gaia_contract.rs:60-68）在两种语义下同值，不构成判别。
建议：补"gt 为含逗号数字"与"乱序列表"两类 golden，或直接落一个对 pinned revision 的 Python 差分测试。M4 的"官方评分"措辞在此之前应保持 docs 现有的保守表述。
M2【中】FINAL ANSWER 提取取到文本末尾
extract_final_answer（headless_bridge.rs:600-609）取最后一个非空标记段直到文本结束："FINAL ANSWER: 42\n\nHope this helps!" 会把闲聊算进答案；**FINAL ANSWER: 42** 的 * 残段在数字题上不被 normalize_number 剥掉 → parse 失败 → INFINITY 哨兵 → 判错。建议按行/段落截断 + 剥首尾 markdown 残段，并补粗体/代码块/尾随文本用例。
M3【中-高，工作流】单题失误永久报废整个 run 的导出资格
失败任务已有 outcome 记录，recover() 的 runnable 集不含它（store.rs:451-454）→ resume 永不重试；叠加 submission 的一票否决（submission.rs:150-154 任何非 Completed 即拒），任何一题 Timeout/missing_final_answer 都使 run 永久失去 submission 资格，只能换 run_id 全量重跑。docs 未提示该不可恢复性。建议：允许 resume 重试失败任务，或 submission 按官方口径"错误计错"导出，至少在 docs 写明。
三、隐私与清理残留
M4【中-高】非 Windows 下 integrity_digest 是无盐明文哈希
DPAPI 绑定密文的修复只覆盖 Windows。Unix 侧 protect 是恒等函数（private_prediction.rs:638-646），blob 内 digest = SHA256(域标签||binding||明文)，而 binding 明文存于同一 envelope、handle 公开在 predictions.jsonl——拿到 blob 即可对 GAIA Level 1 常见短答案做离线字典恢复，对不可猜测答案也是验证预言机。建议：macOS 用 Keychain 派生密钥对称加密，或至少在 docs/gaia-benchmark.md Privacy 节明示该边界。
M5【中】下载路径附件未闭环到硬编码官方 digest
OFFICIAL_LEVEL1_ATTACHMENTS（11 条 path+size+digest 锚）只在 import 路径强制（fetch.rs:771-786、capture_trusted_import_attachments）；下载路径的附件期望 digest 取自 HF ?blobs=true 自报值（parse_hf_metadata → download_one:906-936），全程不引用锚点。低成本修法：download_snapshot 完成后对 11 条 spec 复用同一校验函数交叉复核一次。
M6【低-中】崩溃残留 sessions/eval_* 与 staging 目录无启动清扫
eval 会话列表过滤正确（store.rs:189-193，有测试），但 SIGKILL/断电残留的 sessions/eval_*（含私有题目与附件明文）永久滞留磁盘，boot_inner 无清扫；下载中断的 .pinvou-gaia-tmp-* staging（最大 ~256MB/次）也无 GC；retention 把 eval_ 残留计入普通聊天配额（retention.rs:63 只跳过 sched-）。建议 headless 宿主启动时清扫前任遗留。
四、CI 盲区（B1 漏网的直接原因）
- D1：
cargo test --lib --features benchmark-hooks全仓库任何 job 从未执行——headless_bridge 的 12 个 lib 单测（含 B1 失败用例）不编译不运行。建议在 benchmark-contract 增加这一步。 - D2：pinvou-cli workspace 无 fmt / clippy / cargo-deny 门禁（约 3.5 万行新代码 + parquet/arrow/hf-hub/ureq 等新依赖不在任何 deny 图内）。
- D3：
pinvou-product-backend（唯一把 pinvou3-lib 以 benchmark-hooks 拉进 cli 依赖图的胶水层）在 required 门禁中从不编译；其默认 feature 形态的 pinvou-cli 测试也不跑（required 只跑--no-default-features的 cli 测试）。 - D4：MQ 阶段
benchmark-test永不运行（限定 pull_request 事件），Windows 专属 ACL/DPAPI 代码在 MQ 无覆盖。 - D5（既有缺口，非本 PR 引入，但影响本 PR）：
windows-rust-test不在 required-gate 汇总。
五、范围与卫生
- M13 维持：GUI 聊天 lifetime token 持久化（engine.rs spawn 捕获 + forwarder 终态累加/失败改判 Failed +
persist_chat_token_total）仍混在本 PR，与评测平台无耦合，应拆分为独立 PR 单独评审声明。 - Cargo.lock 陈旧回潮：PR 分叉点早于 #286 的 device_query→x11 收敛，
pinvou3-app/src-tauri/Cargo.lock里device_query+ windows 0.48 链（约 12 个传递依赖）+ agent-client-protocol 系列降级属于基线漂移，请基于最新 main 的 lock 最小化再生。 docs/fork-modifications.md/.en.md在 PR head 中的候选登记段与 gitlink 一致，无冲突标记问题（我在 rebase 时已核对）。
六、底座收尾（流程，好消息 + 顺序敏感）
CodeWhale PR #15 已于今天 10:38 squash merge（d127aed11，单 parent = r7 基线 a36e6cd5，树与候选 a647ed866 完全一致，tree 均为 0da1d356d）。底座 5 个提交的受限轮安全机制深审未发现越权路径，11 条新增 forkguard 测试钉住关键行为。
合并本 PR 前的收尾顺序（顺序敏感）：
- 先在公开 CodeWhale 仓库打不可变标签
pinvou-v0.9.5-r8指向d127aed11（不要指向a647ed866——squash 后两者同树但历史不同，标签应标记已合并的公开历史）； - 本 PR gitlink 切到
d127aed11，同步fork-guard.sh（PUBLISHED_HEAD=d127aed11、PUBLISHED_COMMITS=10、移除候选登记）与verify-public-submodule.sh（PINVOU_CODEWHALE_TAG=r8）； - 不先打 r8 直接入 merge queue 会因严格标签校验失败（当前
--allow-registered-candidate只在 pull_request 阶段放行候选a647ed866）。
合并前条件（建议）
- 修复 B1（一行级）+ B2 + B3，并把
cargo test --lib --features benchmark-hooks纳入 benchmark-contract（D1）； - M1 补两类 golden 或 Python 差分；M2/M3 至少补测试与 docs 说明；
- M4 在 docs 明示 Unix 边界或落地加密；M5 补交叉校验；
- M13 拆分 + Cargo.lock 再生；
- 完成 r8 标签 → gitlink → 脚本三步收尾。
修改面较大，本轮只评审不代修改。这份平台的工程质量整体是高的，前三轮提出的阻断项也已经修掉了绝大部分——把上面这些收口后，期待合入。
当前代码独立评审（基于 head
|
| 现有区域 | 当前 PR 带来的变化 |
|---|---|
| 桌面 UI | 没有评测入口，普通安装用户看不到 GAIA 功能。 |
| 普通聊天 | 开始把整个会话生命周期 token 总数持久化；token 持久化失败时，会把本来完成的聊天回合改报失败（forwarder.rs:648-749）。 |
| 会话删除 | 即使磁盘删除失败，也会清除该会话的运行时 lifecycle、Shell task 和评测模型状态（engine_pool.rs:242-249）。 |
| 会话历史 | 所有 eval_ 前缀会话都从普通历史中隐藏，防止崩溃残留泄露题目；同前缀的非评测会话也会被隐藏（features/sessions/store.rs:180-195）。 |
| 通知 | 缺少 NotificationState 时从“尝试通知”改成“不通知”，避免无窗口宿主 panic；正常 GUI 已管理该状态，预期不受影响。 |
| CodeWhale | 公共 engine/turn loop 有 10 文件、约 +1370/-202 的改动。普通 GUI 仍传 turn_tool_security: None，设计上保留原行为；但这是共享底座状态机，回归面明显大于独立 CLI。 |
| 网络与隐私 | 只有显式 fetch 才访问 Hugging Face，不自动上传结果；Windows 含附件的 GAIA 端到端运行被显式拒绝。 |
附件代码从 app/commands/attachments.rs 迁移到 features/assistant/attachments.rs，并由 lib.rs 保留 re-export，预期只是模块边界调整，不改变普通附件行为。
本轮验证结果
- PR current head：
d9c89dc654eff5083ec8f231f32349ba76fcc008。 - 当前
origin/main：a7c239433d6e5a7ff5c7c5ebbade1caedc9794fa。 - PR 和 main 各自比共同基线前进 23 个提交；
git merge-tree无文本冲突，但尚无当前组合提交的完整 CI。 - 115 个 benchmark/GAIA/core 测试全部通过。
- 16 个 headless bridge 契约测试全部通过。
pinvou-cli --all-features产品后端编译通过。- architecture guard、fork guard、PR 候选 gitlink 校验通过。
- Merge Queue 使用的严格 gitlink 校验确定失败。
- 当前 GitHub required checks 全绿，但
benchmark-test和 Linuxrust-test都是 skipped。
尚未验证：真实 Hugging Face gated 下载、真实模型完整 53 题执行、官方 Python scorer 逐题交叉验证、真实发行包装载，以及当前 main + PR 组合提交的最终 Merge Queue 矩阵。
综上，这个 PR 的核心架构方向和多数安全实现是成立的，但目前仍存在发行闭环、评分状态机、恢复状态机、数据清理生命周期和 Merge Queue gitlink/tag 对齐等明确缺口，修复前不建议合并。
|
复评补充（基于 current head 结论：默认桌面运行时隔离方向已基本成立，但当前仍建议暂不合入。 此前“CLI 未进入桌面安装包”的意见撤回；桌面默认 feature 不启用 仍需阻断合入的问题
外部前置与描述同步
本轮验证
综上：主工程默认运行时隔离基本达到目标；合入前仍应恢复按路径自动运行的最小 CLI 门禁，并修复评分恢复与计划恢复两个状态机问题。 |
|
基于 current head 599d1fe 重新按实际代码复评。本轮按明确目标判断：CLI 独立发布，不进入桌面安装包，并且默认桌面行为不受影响；以下结论不沿用历史评审意见。 结论隔离方向基本成立：当前桌面默认 feature、默认依赖图和 release-packages 打包流程均未纳入 CLI，benchmark-hooks 也已改为可选 feature；普通桌面会话不会注入 benchmark 的逐轮安全限制。此前把 benchmark 行为带入 GUI 的通知、删除、计时等改动，大部分也已恢复或收进 feature gate。 但当前还不能批准合并。剩余问题不是“CLI 是否进入桌面包”，而是独立 CLI 自身存在两个可复现的持久化/恢复正确性问题，CLI 门禁默认不运行，另外默认 GUI 路径仍残留一次无条件 session 读取。独立发布链路和 CodeWhale r8 前置也尚未闭环。 这份 PR 当前实际做什么
已确认通过的隔离边界
阻塞问题1. [P1] CLI 唯一的 CI 验证是 label-only，当前 PR 实际被跳过.github/workflows/pr-check.yml:1033-1037 的 benchmark-contract 和对应 benchmark-test 只在 pull_request 且带 ci:full-benchmark 标签时运行；它们不覆盖 merge_group/main。required-gate 在 1237-1238 又把 skipped 当作可接受结果。 当前 PR 没有该标签，因此 current head 上 benchmark-contract 和 benchmark-test 均为 SKIPPED，而 required-gate 可以成功。常规 Rust jobs 只验证桌面 app，不能替代 pinvou-cli workspace 的编译与测试。也就是说，这个新增独立发布物可以在完全没有被 CI 编译/测试的情况下进入 Merge Queue。 建议把便宜且确定性的 pinvou-cli core/adapter/no-default 检查设为按 pinvou-cli/** 路径触发的必跑门禁，并覆盖 pull_request 与 merge_group；重型 Tauri、Windows、CodeWhale 集成仍可保留为 opt-in/full benchmark。 2. [P1] 运行未完成时生成的 partial score 会锁死最终结果pinvou-cli/crates/cli/src/lib.rs:749-753 允许对未完成 run 评分，并把结果标为 unofficial_partial；随后仍会写入固定的 score.json 和 report.md。运行恢复并完成后，如果最终分数与 partial 内容不同，lib.rs:844-850 会返回 gaia_score_artifact_conflict，而不会更新最终产物。 可复现顺序是：
这使“可恢复运行”和“允许 partial score”两个能力组合后无法产出最终官方结果。应在 run 完成前拒绝发布固定评分产物，或者将 partial/final 分离，并对最终产物提供安全、原子且有版本约束的更新规则。 3. [P1] 任务计划不是原子持久化，崩溃后 resume 可能永久漏题pinvou-cli/crates/benchmark-core/src/service.rs:93-97 先创建 RunStore，再执行 adapter.prepare，之后才逐条写 Planned 事件；RunStore::plan_tasks（store.rs:321-325）也是逐条 append。 因此在 prepare 失败、进程崩溃或写入中断时，磁盘上可能留下空计划或前缀计划。resume_adapter（service.rs:164-166）不会重新准备和校验完整任务集合；store.recover（store.rs:422-454）只能恢复已经持久化的 Planned ID。未写入的题目会从 run 的世界里永久消失，甚至可能把零任务/部分任务运行报告为“剩余为零”。 建议把完整任务计划作为一个原子快照发布，或在 manifest 中绑定 task set/digest；resume 时必须校验计划完整性，并选择修复或明确拒绝恢复，不能把前缀计划当成合法全集。 4. [P2] 默认 GUI 路径仍残留一次无条件 session 读取pinvou3-app/src-tauri/src/features/assistant/engine.rs:1164-1169 现在无论是否 scheduled session 都会调用 store.load 获取 persisted_total_tokens。主线原行为只在 scheduled 场景读取。 虽然本 PR 已移除普通 GUI 的 benchmark token 持久化，但这次无条件读取仍给默认桌面路径增加了磁盘 I/O 和新的失败点，不符合“默认主工程不受影响”的目标。这里应恢复为仅 scheduled 路径读取。 合并/发布前置这些不是上述状态机 bug 的替代项，但仍需要在合并或声明“可独立发布”前闭环：
已公开但仍需保留说明的能力边界
本轮实际验证在 599d1fe 上完成/确认：
最终判断：桌面默认入口和桌面发行包的隔离已经基本通过，但 CLI 正确性、默认 CI 门禁及独立发布闭环尚未通过。建议先修复上述 4 项，并完成 r8/发布前置，再进入批准阶段。 |
|
已基于最新 4 点在
验证结果：benchmark-core 49 项通过，CLI partial-score 定向测试通过，Python CI/隔离策略 53 项通过，Rust format、diff check 与 architecture guard 通过。 当前没有未解决的行级 review thread；以上为顶层评论的逐项处理说明。请基于 |
Signed-off-by: c24894 <chen.zhusen@h3c.com>
Signed-off-by: c24894 <chen.zhusen@h3c.com>
Signed-off-by: c24894 <chen.zhusen@h3c.com>
Signed-off-by: c24894 <chen.zhusen@h3c.com>
Signed-off-by: c24894 <chen.zhusen@h3c.com>
Signed-off-by: c24894 <chen.zhusen@h3c.com>
Signed-off-by: c24894 <chen.zhusen@h3c.com>
b09dd16 to
384c1b8
Compare
|
基于 current head 384c1b8 继续复评。本次 force-push 已把最新 main 纳入分支，GitHub 当前恢复为 MERGEABLE；以下结论只针对该 current head。 结论桌面隔离目标继续成立：默认桌面 feature、默认 Cargo 依赖图和桌面 release-packages 均不包含 pinvou-cli/benchmark product backend，普通会话也不安装 benchmark 逐轮安全策略。本次主线对齐后没有发现新的默认桌面行为回归。 但仍不能批准合并/发布。当前有两个独立 CLI 正确性/可发布性阻塞项，以及 CI、r8 和文档闭环问题。 已修复
仍然阻塞1. [P1] Smoke 的通用恢复路径仍会永久漏 casepinvou-cli/crates/benchmark-core/src/service.rs:66-69 的通用 run 仍逐条写 Planned 事件；如果进程在中途崩溃，磁盘只留下前缀计划。 新增的计划对账只接入 resume_adapter（service.rs:142-166）。Smoke CLI 在 pinvou-cli/crates/cli/src/lib.rs:1075-1097 走的是通用 service.resume，而 service.rs:125-139 直接调用 execute，没有 reconcile_planned_tasks。 因此 Smoke 的可复现路径仍是：计划写到一半崩溃 -> resume -> 只执行已经落盘的前缀 case -> 未落盘 case 永久消失。建议在通用 resume 同样执行计划对账，并增加 service/CLI 调用链级回归测试；当前新增测试只直接覆盖 RunStore helper。 2. [P1] 主线对齐后 pinvou-cli/Cargo.lock 已失配，独立 locked 构建失败在 384c1b8 上执行 cargo test --manifest-path pinvou-cli/Cargo.toml --all-features 时，Cargo 自动改写了 pinvou-cli/Cargo.lock，包括：
恢复提交中的 lockfile 后，实际执行： cargo test --manifest-path pinvou-cli/Cargo.toml --all-features --locked --no-run 会直接失败： cannot update the lock file .../pinvou-cli/Cargo.lock because --locked was passed CLI 既然要单独发布，必须提交与 current main/path dependencies 一致的 lockfile，并把 --locked 构建作为独立发布门禁。否则构建结果依赖现场重新解析，无法复现，正式发布通常也会失败。 3. [P1/发布门禁] 主工程 benchmark jobs 被硬关闭，但没有独立 CLI workflow 承接.github/workflows/pr-check.yml:1034-1039 和 1115-1119 把 benchmark-contract、benchmark-test 固定为 if: false；当前 head 上两项确实均为 SKIPPED，而 required-gate 接受 skipped 并成功。 硬关闭主工程 benchmark jobs 符合“不影响桌面主工程 CI”的方向，但仓库没有独立 CLI check/release workflow，也没有 CLI artifact。建议把 CLI 编译、核心测试、locked build 和发布 smoke 放到独立 workflow，按 pinvou-cli/** 或 CLI release 事件触发，不接入桌面主工程 required-gate。当前保留两段永久关闭的 workflow 代码没有验证价值。 r8 与文档前置
进入 Merge Queue 前仍需发布不可变 r8，并对齐 gitlink、验证脚本、fork 登记及 PR 正文。若 CLI 发布由后续 PR 完成，也应把独立 workflow/artifact 明确列为 release dependency。 本轮验证
最终判断：默认桌面入口和桌面发行包隔离通过；独立 CLI 的 Smoke 恢复、lockfile、独立质量门禁和 r8/发布闭环未通过。建议修完上述项后再批准。 |
Signed-off-by: c24894 <chen.zhusen@h3c.com>
将父仓 gitlink 与公开校验切换到已合并的 r8 不可变标签，并同步中英文 fork 清单、维护策略和升级状态。 移除仅用于 PR 候选期的 gitlink 放行；不启用 benchmark CI 或 CLI 发布链路。 Signed-off-by: hexin <372726039@qq.com>
asto18089
left a comment
asto18089
left a comment
There was a problem hiding this comment.
第五轮复审结论:Approve(独立重审,基于 head 5f8c0f5d)
本轮将 PR 完整 rebase 到最新 origin/main(c75f2fb2)解决全部冲突并逐项重验后,又对作者最新的 r8 对齐提交做了完整核验。结论:平台成立、根因完整交付、无阻断级缺陷,建议合并。
一、根因交付链路逐段核实(成立)
- 端到端接线真实:
pinvou-cli→benchmark-core→adapter-gaia→agent-backend-api(opaque handle / zeroizing secret)→pinvou-product-backend(14 行薄桥,path 依赖 app 的benchmark-hooksfeature)→headless_bridge→EnginePool→ CodeWhale 受限逐轮安全。cargo test --all-features170+ 测试全绿。 - 隐私边界成立:私有预测 handle 为 256-bit 随机(
private_prediction.rs:168),AES-GCM/DPAPI 加密绑定(run_id, task_id, prediction_type, handle)四元组(binding_bytes),Unix 侧 0600/0700 权限在读取路径复检(private_prediction.rs:471/491);公开 outcome 只存 opaque handle。 - 受限工具边界到底:CodeWhale
tool_execution.rs:399-421在最终分发前执行 exact allowlist +is_read_only_for双重门禁;File工具覆写为 action 级判断(file_tool.rs:303,仅 read/list/search_name/search_content),MCP 工具不在 eval 白名单内天然被 exact 门禁拒绝。core(benchmark-core)零 GAIA 泄漏(仅测试字符串),第二适配器(adapter-smoke)证明可复用性是真实的。 - scorer 忠实移植:数字(
parse_python_float+ Unicode 数字折叠 + 下划线规则)/列表(逗号分号逐元素)/字符串(去空白+小写+去标点)三分支对齐官方语义,且文档诚实声明"未做逐题 parity 验证"。
二、隔离边界(上轮的核心争议)——已彻底成立
逐块核实所有 app 侧 diff:GUI 默认构建(--lib --tests 编译验证通过)行为变更实际为零——
- engine_pool/eval 模型快照、
send_eval_user_message、create_empty_with_id、eval_会话前缀过滤(store.rs:194)全部cfg(feature="benchmark-hooks")门控; maybe_notify_task_completed的默认翻转仅 benchmark-hooks 构建生效,默认构建保持unwrap_or(true)原语义(engine_support.rs);- timing/forwarder 的 observation 收尾与
tool_calls/tool_failures/catalog 摘要字段仅对eval_observation_enabled会话生效,普通会话收尾仍是 usage 版; delete_eval_session的失败即 forget + late sweep 仅作用于 eval 会话,delete_chat_session与 main 逐行一致;- attachments 为纯搬移(facade 留在 app/commands,实现在 features),web access-policy 无 eval 入口,发布链路无 CLI 制品。
新版 PR 正文的"隔离边界"一节与代码完全一致。
三、r8 对齐——已验证为真实
pinvou-v0.9.5-r8 标签已发布并解引用到 d127aed1,与 CodeWhale PR #15 squash 合并结果及本 PR gitlink 三者一致(候选链 head a647ed866 与合并树零差异,已逐字节验证);候选期放行逻辑已从 verify-public-submodule.sh 移除,merge queue 严格标签校验现已实际可过。fork-guard(34 条 CodeWhale 指纹 + 65 项 --fast)、architecture-guard、三个 CI 策略测试全绿。
四、验证复现
作者声明的全部命令逐一复现:--locked --all-features 四 crate 子集 153 通过、--no-default-features 22 通过、CodeWhale forkguard_ 34 通过、策略测试 22 通过、verify-public-submodule.sh 通过。锁文件与当前 main 依赖图一致(device_query 退役已反映),--locked 不再触发重解析。
五、遗留观察(均非阻断,记录备查)
PR 正文三处显式声明与代码不符已随正文重写解决。- CodeQL 的 3 条 "Access of invalid pointer" 告警指向已不存在的旧路径(
adapter-gaia/src/fetch/windows_private_acl.rs),实现已移入benchmark-core并以 RAII(LocalFreein Drop)+空指针检查重写,最新 CodeQL 全过——陈旧告警,无需处理。 EnginePool::send_user_message的#[allow(dead_code)]是正确的:消费方product_runtime::submit非 eval 分支在 benchmark-hooks 门控内(mod.rs:247),默认构建下确需该标注。- 已知限制(HF gated 下载未实跑、Windows 附件 fail-closed、product-backend 全量冷编译因磁盘未完成)均已诚实声明;其中第 3 项建议合并后在有富余磁盘的隔离环境补一次完整
--all-features --locked冷编译冒烟。
感谢两位作者的持续响应,五轮迭代后的最终形态质量很高。
Signed-off-by: c24894 <chen.zhusen@h3c.com>
Rebase conflict in timing.rs: #305 introduced finish_turn_internal with ActiveTurnTiming entries and benchmark-hook observation params, but popped the queue front (FIFO). The tail-pop semantics (terminal attributed to the newest queued turn, stale unsubmitted-cancel residue cleared without a terminal, queue cleared whole) is ported into finish_turn_internal; the five observation accessors (record_first_event, tool counters, milestone) now read the queue back to match, so observations land on the live turn. #305's queued_second_turn_survives_single_observation_finish locked "one finish consumes exactly one turn" with FIFO attribution as a side effect; the attribution assertions are adapted to the tail-pop semantics (terminal lands on the newest turn, stale first gets no terminal, emptied-queue finish is a no-op) while keeping the double-finish protection intent. Signed-off-by: asto <asto18089@126.com>
Rebase conflict in timing.rs: #305 introduced finish_turn_internal with ActiveTurnTiming entries and benchmark-hook observation params, but popped the queue front (FIFO). The tail-pop semantics (terminal attributed to the newest queued turn, stale unsubmitted-cancel residue cleared without a terminal, queue cleared whole) is ported into finish_turn_internal; the five observation accessors (record_first_event, tool counters, milestone) now read the queue back to match, so observations land on the live turn. #305's queued_second_turn_survives_single_observation_finish locked "one finish consumes exactly one turn" with FIFO attribution as a side effect; the attribution assertions are adapted to the tail-pop semantics (terminal lands on the newest turn, stale first gets no terminal, emptied-queue finish is a no-op) while keeping the double-finish protection intent. Signed-off-by: asto <asto18089@126.com>
Rebase onto latest main (#305 registered CodeWhale r8) exposed stale facts and a few audit leftovers; all comment/doc-only: - THIRD_PARTY_NOTICES: CodeWhale baseline r7 -> r8 (gitlink d127aed11) - fork-modifications(.en)/fork-policy(.en): reconcile dates with the 2026-08-24 r8 registration; fix forkguard count 23 -> 34 in section 4 (stale on main since #305) - codewhale-upgrade report: snapshot note now describes section 9 as living text (PR #285 -> PR #305) instead of the pre-#305 reading - acceptance checklist: drop dangling section 8.2 reference in row 3.2 - codex-acp: collapse inline npm/brew upgrade commands into the multi-agent-acp.md link (dedup the PR itself introduces) - plugin-package-spec section 10: MCP id charset no longer claims lowercase enforcement the importer only applies to package ids - plugin-protocol section 13.1: dependency-download row status now matches sections 3/10 (plugin.json dependencies key unconsumed) - run-mac-verify.sh comment: point to pinvou3-app/INSTALL.md instead of a nonexistent README prerequisites section - release-packages.yml comment: only package-lock.json is existence-gated; the other five version files are required Signed-off-by: asto <asto18089@126.com>
Rebase onto latest main (#305 registered CodeWhale r8) exposed stale facts and a few audit leftovers; all comment/doc-only: - THIRD_PARTY_NOTICES: CodeWhale baseline r7 -> r8 (gitlink d127aed11) - fork-modifications(.en)/fork-policy(.en): reconcile dates with the 2026-08-24 r8 registration; fix forkguard count 23 -> 34 in section 4 (stale on main since #305) - codewhale-upgrade report: snapshot note now describes section 9 as living text (PR #285 -> PR #305) instead of the pre-#305 reading - acceptance checklist: drop dangling section 8.2 reference in row 3.2 - codex-acp: collapse inline npm/brew upgrade commands into the multi-agent-acp.md link (dedup the PR itself introduces) - plugin-package-spec section 10: MCP id charset no longer claims lowercase enforcement the importer only applies to package ids - plugin-protocol section 13.1: dependency-download row status now matches sections 3/10 (plugin.json dependencies key unconsumed) - run-mac-verify.sh comment: point to pinvou3-app/INSTALL.md instead of a nonexistent README prerequisites section - release-packages.yml comment: only package-lock.json is existence-gated; the other five version files are required Signed-off-by: asto <asto18089@126.com>
Rebase conflict in timing.rs: #305 introduced finish_turn_internal with ActiveTurnTiming entries and benchmark-hook observation params, but popped the queue front (FIFO). The tail-pop semantics (terminal attributed to the newest queued turn, stale unsubmitted-cancel residue cleared without a terminal, queue cleared whole) is ported into finish_turn_internal; the five observation accessors (record_first_event, tool counters, milestone) now read the queue back to match, so observations land on the live turn. #305's queued_second_turn_survives_single_observation_finish locked "one finish consumes exactly one turn" with FIFO attribution as a side effect; the attribution assertions are adapted to the tail-pop semantics (terminal lands on the newest turn, stale first gets no terminal, emptied-queue finish is a no-op) while keeping the double-finish protection intent. Signed-off-by: asto <asto18089@126.com>
Rebase onto latest main (#305 registered CodeWhale r8) exposed stale facts and a few audit leftovers; all comment/doc-only: - THIRD_PARTY_NOTICES: CodeWhale baseline r7 -> r8 (gitlink d127aed11) - fork-modifications(.en)/fork-policy(.en): reconcile dates with the 2026-08-24 r8 registration; fix forkguard count 23 -> 34 in section 4 (stale on main since #305) - codewhale-upgrade report: snapshot note now describes section 9 as living text (PR #285 -> PR #305) instead of the pre-#305 reading - acceptance checklist: drop dangling section 8.2 reference in row 3.2 - codex-acp: collapse inline npm/brew upgrade commands into the multi-agent-acp.md link (dedup the PR itself introduces) - plugin-package-spec section 10: MCP id charset no longer claims lowercase enforcement the importer only applies to package ids - plugin-protocol section 13.1: dependency-download row status now matches sections 3/10 (plugin.json dependencies key unconsumed) - run-mac-verify.sh comment: point to pinvou3-app/INSTALL.md instead of a nonexistent README prerequisites section - release-packages.yml comment: only package-lock.json is existence-gated; the other five version files are required Signed-off-by: asto <asto18089@126.com>
* docs: repo-wide documentation truthfulness audit Audit all project-owned docs (root governance, README bilingual, fork governance, plugin/marketplace specs, ACP/code-mode, knowledge domain, remote control/ADR, frontend and Rust architecture READMEs, scripts and CI comments) against the actual code, and fix the confirmed findings: - README bilingual: remove unimplemented in-app update/OTA claim (only unsupported stubs exist), correct macOS target to universal builds, React 18->19 badge, complete model preset and repository layout lists - INSTALL.md: replace hardcoded 0.6.2 deb name with the actual release artifact naming, add Ubuntu 22.04+/glibc 2.35+ floor, complete preset list - THIRD_PARTY_NOTICES: CodeWhale baseline pinvou-v0.9.0-r4 -> r7, fix <os>/<arch> path template; lark NOTICE visual-design location - fork governance: correct the gitlink bump attribution (Pinvou#285, not Pinvou#302), rewrite acceptance checklist references to retired v0.8 forkguard tests, unify net-lines drift metric, sync lagging .en docs, add historical-snapshot note to the 0.9.5 upgrade report - plugin protocol/spec: flat mcp/ layout (dir must be exactly 'mcp'), mark archive.zip and credentials/config_fields/dependencies as unimplemented, complete the import rejection rules incl. same-id update semantics, server.py hard constraint, DenyAll default note - code-native-agent: add revision notes to §8.3/§8.6/§8.7 superseded by disabled_bundles.json and three-lane mode persistence; fix §8.6->§8.7 cross-reference; dedupe codex-acp against multi-agent-acp - remote control: fix phase1 doc file names (manager/, relay_client.rs), drop dangling ADR-0001 reference; PROTOCOL.md documents credential stripping, 24 error codes and desktop_endpoint_replaced (4001) - ADR: add directory README (0001-0005 history, terminology mapping), rename 0010/0013 to match their titles, cross-link 0009/0019, 0010/0022 - Rust/frontend architecture READMEs: remove stale include!/#[path] migration description, nonexistent AttachmentStager, complete resources/platforms tree, codex-bridge official-install wording, updater stub comment, Cargo.lock 0.8.5->0.8.6 - governance: DCO.md matches actual CI behavior (presence check, merge commit exemption), zh CONTRIBUTING precedence note, document the local commit-msg hook, dedupe AGENTS/CONTRIBUTING fork-boundary and language-rule duplication Verification: sync-version --check, architecture-guard, fork-guard --fast, relay tests 23/23, frontend node tests 232/232 (2 skipped by design), cargo check --locked, cargo test --lib 1393 passed (single- threaded). Signed-off-by: asto <asto@pinvou.com> Signed-off-by: asto <asto18089@126.com> * docs: correct audit errata and close gaps Review follow-up for the repo-wide documentation audit (second-party verification of ~150 claims; 96% verified, no fabrications): - INSTALL.md: gemini preset goes through the official OpenAI-compatible endpoint (bridge.rs routes it on the openai wire), not a separate channel; keep the anthropic note as is - CONTRIBUTING (en/zh): restore the conflict-resolution rule deleted from AGENTS.md (preserve compatible functionality from both sides, never silently pick between behaviorally different alternatives) so the AGENTS.md pointer to a single source of truth is no longer hollow - fork-modifications.md: the gitlink/r7-tag identity still holds, but pinvou3-clean HEAD moved to d127aed1 via CodeWhale PR Pinvou#15 (r8 in flight, to be registered by the r8 sync PR); also record the Pinvou#302 parent-side switch to the single disabled_bundles.json so the authoritative Chinese register matches the English summary - remote-control-phase1-architecture.md: exclusive subscriptions, event sequencing, and replay state live in manager/ (mod.rs StreamState + rpc.rs helpers); relay_client.rs is the pure WebSocket transport layer (connect/reconnect backoff, heartbeat, backpressure) - release-packages.yml comment: sync-version.mjs does not check pinvou3-app/src-tauri/Cargo.lock (hence the manual bump in this PR); package-lock.json is included only when present - run-mac-verify.sh: list p7zip in the probe comment (the loop still checks it) - README (en/zh): built-in template list also covers Anthropic and Gemini, matching model-catalog.js - sbom.md: drop the dangling reference to a nonexistent manual release checklist; state the SBOM duty directly Verification: sync-version --check, architecture-guard, fork-guard --fast, cargo check --lib --locked all pass; bash -n on the touched script. Signed-off-by: asto <asto18089@126.com> * docs: fix residual audit inaccuracies Third-party re-verification of the whole PR against code, git history, CI, and the release pipeline found five factual errors introduced by the audit itself, plus six residual stale spots; all fixed in place: - plugin-package-spec Pinvou#11-9: 'uninstall then re-import' cannot clear the same-id conflict - uninstall deliberately retains bundles/<id>/ for Upload-source packages (user's only copy, mod.rs); corrected to package-id change or manual directory removal - marketplace-unification-todo: the rewritten authority claim matched the write path only; the read path already flipped to BundleStore (bundle.rs installed-truth reversal, installed.json as corruption fallback; legacy list_marketplace_tools view and frontend fallback noted) - code-native-agent: 18 is the NATIVE_CHAT_EVENTS subscription list length; the lane reducer consumes 19 chat:* events - both lines now attribute the counts correctly - CONTRIBUTING.zh-CN: sync the merge-commit DCO exemption into the Chinese DCO sentence (en/zh parity) - macos codex-bridge README: missing Codex CLI on macOS goes through the official install script (like Linux); brew only upgrades an existing brew-managed install - residuals: entitlements.plist mac.signingIdentity -> macOS.signingIdentity; codex-acp.md release section now routes upgrades by install source per multi-agent-acp.md matrix; L1-judge-rubric stale illustrative numbers (r1 example, 4-item template note, 4->5 dim bump example); fork-modifications r8 wording (CodeWhale tag already published, parent registration pending); prepare-codex-bridge-runtime.sh comment drops the stale 'managed Codex' phrase Verification: sync-version --check, architecture-guard, fork-guard --fast, bash -n on the touched script, plutil -lint on entitlements.plist. Signed-off-by: asto <asto18089@126.com> * docs: fix rebase-stale facts and dangling refs Rebase onto latest main (Pinvou#305 registered CodeWhale r8) exposed stale facts and a few audit leftovers; all comment/doc-only: - THIRD_PARTY_NOTICES: CodeWhale baseline r7 -> r8 (gitlink d127aed11) - fork-modifications(.en)/fork-policy(.en): reconcile dates with the 2026-08-24 r8 registration; fix forkguard count 23 -> 34 in section 4 (stale on main since Pinvou#305) - codewhale-upgrade report: snapshot note now describes section 9 as living text (PR Pinvou#285 -> PR Pinvou#305) instead of the pre-Pinvou#305 reading - acceptance checklist: drop dangling section 8.2 reference in row 3.2 - codex-acp: collapse inline npm/brew upgrade commands into the multi-agent-acp.md link (dedup the PR itself introduces) - plugin-package-spec section 10: MCP id charset no longer claims lowercase enforcement the importer only applies to package ids - plugin-protocol section 13.1: dependency-download row status now matches sections 3/10 (plugin.json dependencies key unconsumed) - run-mac-verify.sh comment: point to pinvou3-app/INSTALL.md instead of a nonexistent README prerequisites section - release-packages.yml comment: only package-lock.json is existence-gated; the other five version files are required Signed-off-by: asto <asto18089@126.com> * docs: correct importer claims and plan_resolved wording - plugin-package-spec.md: the manifest-driven import path validates only the package id charset; MCP/Skill component ids get no charset check and the Skill frontmatter is not parsed, so document the actual enforcement and mark the consistency/charset gaps as known and separately tracked. The unix_mode check matches symlinks only (S_IFLNK), not hardlinks. - code-native-agent.md: NATIVE_CHAT_EVENTS subscribes to 18 chat:* events; chat:plan_resolved is emitted at runtime by discard_plan (interaction.rs) and consumed on the bridge path, but it is absent from NATIVE_CHAT_EVENTS, so the lane's switch branch is unreachable through this subscription and the consumed count stays 18, consistent with the architecture table. Signed-off-by: asto <asto18089@126.com> --------- Signed-off-by: asto <asto@pinvou.com> Signed-off-by: asto <asto18089@126.com>
Rebase conflict in timing.rs: #305 introduced finish_turn_internal with ActiveTurnTiming entries and benchmark-hook observation params, but popped the queue front (FIFO). The tail-pop semantics (terminal attributed to the newest queued turn, stale unsubmitted-cancel residue cleared without a terminal, queue cleared whole) is ported into finish_turn_internal; the five observation accessors (record_first_event, tool counters, milestone) now read the queue back to match, so observations land on the live turn. #305's queued_second_turn_survives_single_observation_finish locked "one finish consumes exactly one turn" with FIFO attribution as a side effect; the attribution assertions are adapted to the tail-pop semantics (terminal lands on the newest turn, stale first gets no terminal, emptied-queue finish is a no-op) while keeping the double-finish protection intent. Signed-off-by: asto <asto18089@126.com>
Rebase conflict in timing.rs: #305 introduced finish_turn_internal with ActiveTurnTiming entries and benchmark-hook observation params, but popped the queue front (FIFO). The tail-pop semantics (terminal attributed to the newest queued turn, stale unsubmitted-cancel residue cleared without a terminal, queue cleared whole) is ported into finish_turn_internal; the five observation accessors (record_first_event, tool counters, milestone) now read the queue back to match, so observations land on the live turn. #305's queued_second_turn_survives_single_observation_finish locked "one finish consumes exactly one turn" with FIFO attribution as a side effect; the attribution assertions are adapted to the tail-pop semantics (terminal lands on the newest turn, stale first gets no terminal, emptied-queue finish is a no-op) while keeping the double-finish protection intent. Signed-off-by: asto <asto18089@126.com>
* fix: memory leaks and object reuse repo-wide
Whole-repo memory audit (frontend, src-tauri, platform layer) with
safe, behavior-preserving fixes; CodeWhale submodule findings are
reported upstream-first and intentionally untouched here.
Unbounded growth (P0):
- Session working-set buffers only pruned scheduled sessions; normal
sessions stayed resident for app lifetime. Add an all-session LRU
(cap 96, protects active/busy/queued/remote-turn buffers) on both
tauri and web bridges; evicted buffers rehydrate from disk.
- render_artifact_visual cache held multi-MB payloads keyed by
path|mtime forever. Bound it with an LRU (16 entries / 96 MiB,
same-path stale keys yield on insert).
- KnowledgeView output-preview ref cache grew per browsed artifact;
cap at 48 entries.
- Startup timing entries in index.html appended forever; keep last 64
and keep the flush cursor consistent.
Structural leaks (P1):
- TurnLifecycle transcript rules accumulated one full raw prompt +
display clone per turn for the engine lifetime. Resynced engines are
seeded with already-sanitized history, so prune rules after a
successful SyncSession (active turn guarded); regression test added.
- timing turn queue: unsubmitted-cancel paths left stale ids that the
next finish_turn misattributed; pop from the tail and clear the
session queue, plus clear_session on session delete.
- codex_acp event bridge kept terminal tool_call entries with full
raw payloads in the tools map; remove on terminal notification.
- Web domain-adapter built fresh slices per notify, re-rendering every
domain subscriber on any change; cache (full, slice) per subscriber
and keep the flat/domain subtree-sharing contract (test updated).
- Unix zombies: open/xdg-open/notify-send spawned without wait; add a
shared detached reaper helper with tests.
- Reuse heavy clients: shared SystemCredentialStore handle (bridge),
shared reqwest clients for monitor probes and IMA connector with
per-request timeouts.
Hygiene (P2/P3): notification dedup set bounded per session, credential
delete removes cache and key-lock entries, modeStateEpochs purged with
session buffers, SearchView group refs deleted on unmount, cached
Intl date formatters, pet window skips identical 600ms ticks and
memoizes markdown cards.
Verification: cargo test --lib 1396 passed / 0 failed (2 stale contract
tests updated to the new behavior); node --test suite has zero new
failures versus the pre-existing sandbox baseline; architecture-guard
passes; no submodule changes.
Signed-off-by: asto <asto18089@126.com>
* fix: cache capacity and eviction gaps
Second-pass cache audit (capacity + necessity lenses, 12 read-only
subagent sweeps over frontend bridges, src-tauri, and CodeWhale).
This commit lands the findings that affect code changed in this PR
plus small same-area gaps; larger redesigns and CodeWhale findings
are reported for follow-up instead.
- toolMeta (tauri bridge): historical tool_use metadata written during
rerenderFromMessages was never cleared and rode into session buffers
(write/patch args can be hundreds of KB). Clear it at replay start;
the live-event path stays insert/delete balanced.
- outPreviewCache (KnowledgeView): count-only cap missed the byte axis
(48 multi-MB office HTML entries could reach hundreds of MB). Add a
32 MiB cumulative budget with insertion-order eviction; count cap 48
kept as secondary axis.
- MAX_SESSION_BUFFERS 96 -> 32 on both bridges: heavy sessions run
1-4 MB each and real switching hot sets are single-digit, so 96 paid
worst-case hundreds of MB for near-zero hit rate; eviction falls back
to a one-time disk rehydrate.
- Prune-path gaps: the scheduled-buffer LRU skipped the
onSessionBufferPurged hook (leaking modeStateEpochs keys) and both
LRU loops missed personaPlaceholderTitles; scene-events localStorage
keys are now removed on session purge/eviction (tauri via the purge
hook, web inline in all three paths).
- visual_cache key now uses millisecond mtime (same-second rewrites
previously returned a stale preview) and gains unit tests for
same-path eviction, entry cap, and byte budget.
- pending_user_input cleared on session delete alongside the existing
timing/turn-capture cleanup.
Verification: cargo test --lib 1399 passed / 0 failed; frontend
node --test zero new failures vs the pre-existing baseline;
architecture-guard passes.
Signed-off-by: asto <asto18089@126.com>
* fix: review follow-ups for eviction and cleanup paths
Review follow-ups for the memory-leak audit fixes:
- Tauri bridge fast-path switch: an event listener recreates an
unloaded empty buffer via getBuffer for any session a late event
names (chat:usage/artifact:disk after eviction). Switching to such
a buffer showed an empty conversation because the cached fast path
only checked buffer existence. Gate the fast path on
loadedFromDisk/busy/remoteTurnActive/content (web bridge already
gates on loadedFromDisk) so poisoned buffers fall through to the
durable reload.
- rerenderFromMessages toolMeta wipe: the live-session hydration path
(hydrateLiveSession) restores toolMeta from the live buffer and then
rerendered, wiping entries for in-flight tools whose tool_use is not
yet in messages; the subsequent chat:tool_end then missed its meta
(stuck request_user_input card, degraded present_artifact card).
Add opts.keepLiveToolMeta and pass it on the live-hydration call
site (tauri + web). The web bridge also gains the durable-replay
toolMeta reset that only tauri had, closing the same leak there.
- posix spawn_detached_and_reap: the doc claimed a synchronous-wait
fallback on reaper-thread failure, but the Child had been moved into
the dropped closure, so no wait happened and callers surfaced a
spurious error for a successfully spawned command. Route the Child
through a shared Option slot so the failure path can wait on the
caller thread, and return Ok (the command did launch).
- Shared reqwest clients: Client::builder().build().unwrap_or_default()
re-runs an identical build on failure and panics (Client::default()
expects). ima.rs keeps per-call error propagation; model_probe
returns None so the probe falls back to configured values, matching
the pre-shared-client degradation.
- Web session-creation rollback (remote_control): clear timing queue
and pending_user_input alongside store.delete, same as delete_session.
- Comment accuracy: onSessionBufferPurged has no return-value
protocol; eviction also drops buffer-resident drafts (not "pure
presentation cache"); credential KEY_LOCKS doc notes the delete-time
registration exception; stale test renamed to the asserted behavior.
Verification: cargo test --lib 1396 passed / 3 failed (the three
codex_acp attachments tests are the known /tmp-worktree environment
failures, unchanged by this commit); frontend npm test 232 passed /
0 failed / 2 skipped; architecture-guard and fork-guard --fast pass.
Signed-off-by: asto <asto18089@126.com>
* fix: harden eviction gates and rule pruning
Independent re-review fixes (round 3) on top of the audit fixes:
- Fast-path gate: drop the chatItems clause. A non-turn event
(chat:compaction) recreating an empty buffer via getBuffer only
adds a system chatItem without setting busy or messages; admitting
it on the fast path showed a history-less view with no self-heal.
Gate on loadedFromDisk/busy/remoteTurnActive/messages only.
- switchActiveTo: set the new activeSessionId before touching the
old buffer. The touch-triggered LRU prune protects the target via
activeSessionId; with the old ordering an idle target could be
evicted mid-switch and silently replaced by freshBuffer() (both
bridges).
- Transcript rule pruning: replace the state.active bail (a no-op on
every interactive path — reserve() always precedes spawn) with a
retain that keeps only the in-flight reservation's rule, and prune
stale rules on engine reclaim so rules no longer outlive the engine
in turn_lifecycles until session delete. Test rewritten to lock the
production shape (stale rules cleared while a reservation is held).
- credential_store: deregister KEY_LOCKS entries only when no other
thread still holds the lock handle (Arc::strong_count == 2), so a
concurrent get/set can no longer build a second lock and lose
mutual exclusion against an in-flight op.
- visual cache: parse keys with rsplit_once('|') — filenames may
contain '|' and left-splitting evicted unrelated entries.
- PetWindow: include body in sameActivities — copy-derived bodies
change on language switch without any event field changing.
- domain-adapter: comment accurately scopes the stablePick cache
(whole-snapshot granularity, weaker than the desktop per-domain
revision cache).
- cargo fmt on model_probe.rs/ima.rs (rust-lint gate).
cargo test --lib green (one pre-existing runtime_bundle flaky passes
in isolation); npm test 5 failures identical to the pre-PR sandbox
baseline; architecture-guard passes.
Signed-off-by: asto <asto18089@126.com>
* fix(web): stabilize subscribeMany combined slice identity
The stablePick cache stabilized each per-domain slice but the combined
outer object passed to subscribeMany callbacks was rebuilt (and frozen)
on every notification. The only domain consumer is a React setState
(useBridge.js), which bails out solely on Object.is identity, so every
no-change notify still caused a full re-render and the re-render churn
this change set out to eliminate was unaffected.
Memoize the combined object on the transport snapshot reference (same
single-slot pattern as stablePick) and lock the behavior with a
discriminating contract test (identity reuse on a no-change notify, new
identity on a real change).
Signed-off-by: asto <asto@users.noreply.github.com>
* fix(chat): keep drafts and scene keys on eviction
Fifth-round review findings on the session-buffer LRU:
P1 - Evicting an idle working set discarded its composerDraft. The
disk transcript only holds committed content (neither bridge persists
drafts), so after 33 sessions the unsent draft silently became
unrecoverable. Eviction now stashes non-empty drafts into a bounded
side table (256 entries, strings only) and every buffer-rebuild path
(getBuffer, switchActiveTo fallback, slow-path rehydrate, hydrateLive
entry) restores it; real session deletion (purgeSessionBuffer)
invalidates the stash so it never flows back into a recycled id.
P2 - LRU eviction removed the scene-events localStorage key, which is
the only offline recovery copy when the sidecar save fails (that
failure is intentionally swallowed and sync relies on the cache to
replay). Eviction no longer touches the key; only real session
deletion cleans it (tauri via the new reason="delete" purge hook,
web via purgeSessionBuffer only).
Adds tests/session_buffer_eviction.test.mjs: 33-session eviction with
draft restore and bounded buffer count (tauri+web), protection
predicates, stash bounds, delete-vs-evict semantics, and the
save-fail + evict + rehydrate scene recovery path (web, driven through
public switchToSession/setComposerDraft/deleteSession). All 7 tests
fail on the pre-fix head.
Signed-off-by: asto <asto18089@126.com>
* fix: bound draft stash and reap browser spawn
Sixth-pass review fixes:
- Cap stashed eviction drafts at 64K chars (both bridges): the side
table promised short strings only, but external transport callers
bypass the composer input limit, so 256 unbounded strings could
re-introduce the retained-memory leak the PR removes.
- Reap the agent-login browser spawn via the shared posix reaper,
exposed through platform::os::spawn_detached_and_reap (unix reaps,
windows has no zombie contract). open/xdg-open/notify-send were
fixed earlier; this browser launch was the missed same-class site.
- Lock the timing queue semantics with discriminating tests (tail-pop
attribution + clear_session silencing late finish).
- Extend the eviction suite: scheduled-run reopen after eviction and
late chat:usage getBuffer rebuild both restore the stashed draft
(mutation-verified discrimination).
Verified: cargo test --lib 1377 passed (1 pre-existing runtime_bundle
flaky, green in isolation); node --test 243/250 (5 failures identical
to pre-PR baseline); cargo fmt, architecture-guard, commit gate pass.
Signed-off-by: asto <asto18089@126.com>
* fix: rebase onto #305 and adapt timing tests
Rebase conflict in timing.rs: #305 introduced finish_turn_internal
with ActiveTurnTiming entries and benchmark-hook observation params,
but popped the queue front (FIFO). The tail-pop semantics (terminal
attributed to the newest queued turn, stale unsubmitted-cancel
residue cleared without a terminal, queue cleared whole) is ported
into finish_turn_internal; the five observation accessors
(record_first_event, tool counters, milestone) now read the queue
back to match, so observations land on the live turn.
#305's queued_second_turn_survives_single_observation_finish locked
"one finish consumes exactly one turn" with FIFO attribution as a
side effect; the attribution assertions are adapted to the tail-pop
semantics (terminal lands on the newest turn, stale first gets no
terminal, emptied-queue finish is a no-op) while keeping the
double-finish protection intent.
Signed-off-by: asto <asto18089@126.com>
* fix: harden ACP turn timing and retention purge gaps
Round-7 review fixes:
- ACP start_turn now runs only after send_message's busy admission
succeeds (codex.rs). ACP has no native-lane reserve/terminal_closing
gate, so a concurrent second submit failing admission used to leave a
ghost queue entry behind; its send_error finish then cleared the
whole queue and swallowed the in-flight turn's assistant_done.
- Startup marks cursor no longer counts spliced-out entries: the flush
success callback relocates `sent` by the in-flight batch tail's
identity, so late real entries are never short-circuited as already
reported (index.html). Comment now states the web-end (sent stays 0)
truncation semantics honestly.
- visual_cache key parsing regression test for filenames containing
'|' (artifacts.rs): left-split would make same-prefix files evict
each other's entries.
- Retention-driven session deletions now clear process-level turn
state maps via a SessionPurgedHook registered at the composition
root (dependency inversion: sessions must not depend on assistant).
Covers chat retention, scheduled cleanup, delete_scheduled_run and
purge_all_scheduled_side_maps; also prevents late finish_turn from
rebuilding orphan timing sidecars outside deleted session dirs.
Verification: cargo test --lib 1405 passed / 3 failed (pre-existing
/tmp-worktree codex_acp attachments env failures, identical on the
pre-fix head); new tests retention_purge_notifies_session_purged_hooks
and pipe_in_filename_parse... pass; timing suite 16/16; architecture
guard passes; affected node suites green.
Signed-off-by: asto <asto18089@126.com>
* fix(chat): stop eviction-time draft and title loss
Review follow-ups on the session-buffer LRU:
- tauri switchActiveTo fresh branch now marks the materialized empty
buffer loadedFromDisk (web bridge parity). Without it the new
cachedBufferUsable gate rejected fresh persona/ensureSession buffers,
the slow path replaced them via freshBuffer() without stashing, and
unsent composer drafts were silently discarded.
- personaPlaceholderTitles is session metadata, not buffer residue, and
no rehydrate path restores it; capacity eviction must keep it. Only
real session deletion (purgeSessionBuffer) clears it now, on both
bridges.
- New regression tests: fresh-buffer draft survival (mutation-verified),
placeholder-title eviction/delete semantics, and the 64K draft-stash
char bound on both bridges.
- Correct stale comments: 32-buffer cap arithmetic, draft char-limit
wording, and the toolMeta replay-residue description (replay re-adds
historical entries; the LRU bounds their retention).
Signed-off-by: asto <asto18089@126.com>
* fix(artifacts): skip over-budget insert, reap soffice
- visual_cache_insert: a single result larger than the 96 MiB byte
budget previously wiped the whole cache including itself via the
eviction loop. Over-budget results are now skipped entirely (the
caller response was already cloned before insert). Covered by a new
mutation-verified unit test.
- open_with_libreoffice still spawned soffice without reaping (Unix
zombie per invocation); route it through the shared
platform::os::spawn_detached_and_reap helper like the other
fire-and-forget launches.
Signed-off-by: asto <asto18089@126.com>
* fix(assistant): clear timing state on eval teardown
delete_chat_session_with_gate (used by eval teardown and
ProductChatRuntime::close) left unpaired timing queue keys behind;
a GAIA pass creates/deletes ~165 sessions, so ACTIVE_TURNS grew
unboundedly. Clear the timing state on delete (idempotent with the
SessionPurgedHook cleanup). Adds a mutation-verified regression test
and renames the stale tail-pop queue test to match its semantics.
Signed-off-by: asto <asto18089@126.com>
* refactor(sessions): fire purge hooks from store delete
SessionStore::delete now fires notify_session_purged after all store
locks are dropped, so every deletion path (delete_session command,
remote-control rollback, retention purge) gets the same process-level
cleanup instead of hand-duplicated clears at two call sites. The hook
registered in lib.rs additionally clears memory turn captures and the
monitor self-metrics keys (warmed_sessions grew per session and were
never reclaimed). Hooks remain optional; unregistered deletion proceeds
unchanged.
Signed-off-by: asto <asto18089@126.com>
* docs: align cache and reaper comments with behavior
- posix: the reaper rationale now covers the long-lived browser case
(agent-login first instance parks the reaper thread until exit).
- credential_store: document that delete no longer leaves a cached
None, so the first post-delete get re-touches the keyring backend
once per process (trade against the #175 anti-popup cache).
- ima/model_probe: document that the OnceLock-shared reqwest client
snapshots the system proxy at first build (mid-session proxy changes
need a restart) and caches build failure process-wide.
Signed-off-by: asto <asto18089@126.com>
* refactor(notifications): call reaper via os interface
send_notify_send reached into platform::os::posix directly; use the
unconditional platform::os::spawn_detached_and_reap wrapper introduced
in this PR so callers stay off platform internals (same idiom as the
codex_acp agent-login call site).
Signed-off-by: asto <asto18089@126.com>
* fix(sessions): refuse eviction instead of dropping unsent drafts
The draft stash's bounds (256 entries, per-draft char cap) previously
silently discarded user input: an over-cap draft or the 257th drafted
session was dropped at eviction, and legitimate UI drafts between 64K
and the composer's 100K input cap were also lost (the old cap was
below the UI limit). Both bounds now refuse the eviction instead —
the buffer stays resident and the draft survives; the char cap rises
to 1M (10x the composer input cap). Tests assert recovery for
oversized and capacity-boundary drafts and are mutation-verified
(reverting the refusal turns them red).
Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
* chore: translate PR-added comments to English
Per AGENTS.md, new code comments and diagnostics must be English.
Translates all 460+ PR-added Chinese comment blocks and assertion
messages across the Rust platform/feature layers, both bridges, the
frontend components, and the eviction regression suite (pre-existing
localized strings merely moved by refactors are exempt and untouched).
No behavior change; the eviction suite header also documents the new
refuse-eviction stash semantics.
Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
* fix(codex): register ACP timing turn inside admission
Move the timing turn registration from the caller (after send_message
returns) into the admitted section of AcpPool::send_message, immediately
before the prompt task is spawned, via the new begin_prompt_turn gate
helper.
The spawned prompt task can complete (fast mock response, instant
connection error) and call timing::finish_turn before send_message
returns, so a caller-side registration after the await raced that
finish: the completion found an empty queue and was dropped, while the
late start_turn left a stale unpaired queue entry attributed to the
next turn. Registering after busy admission succeeds also keeps the
round-7 invariant: a rejected concurrent submit never enqueues a ghost
turn whose send_error finish would clear the in-flight turn's queue.
Adds two regression tests (operation_gate): immediate completion is
recorded on the registered turn with the queue left empty, and a busy-
rejected submit neither enqueues a ghost entry nor swallows the
in-flight terminal. Both are mutation-verified (registration removed /
moved before admission each turn the suite red).
Signed-off-by: asto <asto18089@126.com>
* chore: collapse doubled comment markers from translation
Normalize the malformed comment formatting left by the English
translation pass: comment continuation lines carried duplicated
markers and indentation artifacts ("/// ///", "// //",
"/// ///"), which produced malformed rustdoc and hurt readability.
The text itself is unchanged; only the repeated marker prefixes are
collapsed to a single marker. 228 affected lines across 19 files,
verified to leave no residue and to touch no code lines.
Signed-off-by: asto <asto18089@126.com>
* fix(codex): make ACP admission atomic across touch failure
Signed-off-by: asto <asto18089@126.com>
* test(codex): update activity-touch contract for admission gate
Signed-off-by: asto <asto18089@126.com>
* chore: translate round-9 contract comment to English
Signed-off-by: asto <asto18089@126.com>
---------
Signed-off-by: asto <asto18089@126.com>
Signed-off-by: asto <asto@users.noreply.github.com>
Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
Co-authored-by: asto <asto@users.noreply.github.com>
Co-authored-by: asto <asto@local>
4 participants
背景
项目需要一套可复用的本地 benchmark 平台，并以固定版本的 GAIA validation Level 1 作为首个官方适配器，同时保证私有题目、附件、预测和提交文件不会进入公开运行产物或越过受限工具边界。
变更
pinvou-cliworkspace：benchmark core、Smoke 适配器、GAIA 适配器、产品 backend 与 typed CLI。Fileaction 投影、ShellPolicy::ReadOnly和最终分发复检。gaia-final/v1契约；缺失最终答案时记录missing_final_answer，完整运行评分后才发布report.md与score.json。d127aed113529dc93754d044b9f352e9746f6b83，并发布为不可变标签pinvou-v0.9.5-r8；父仓 gitlink、fork guard 与中英文 fork 文档均已对齐。隔离边界
pinvou-cli是独立 workspace，不加入桌面 Tauri workspace。benchmark-hooks；主工程发布 workflow 不包含 CLI 制品。验证
cargo test --manifest-path pinvou-cli/Cargo.toml --locked --all-features -p agent-backend-api -p benchmark-core -p adapter-smoke -p adapter-gaia：153 个测试通过。cargo test --manifest-path pinvou-cli/Cargo.toml --locked -p pinvou-cli --no-default-features：22 个测试通过。cargo test -p codewhale-tui --lib --locked forkguard_ -- --test-threads=1：34 个测试通过。python3 -m unittest scripts.tests.test_ci_gate_policy scripts.tests.test_benchmark_isolation_policy scripts.tests.test_headless_boot_contract：22 个测试通过。./scripts/verify-public-submodule.sh、./scripts/fork-guard.sh --fast、python3 scripts/architecture-guard.py、格式和 diff 检查：通过。已知限制
attachments_platform_security_unsupported，当前不声明 Windows GAIA 端到端实跑支持。--all-features --locked冷编译在本机因磁盘空间耗尽未完成；锁文件解析、core/adapter/无 product-backend 测试及 CodeWhale 安全回归均已完成。