Skip to content

fix: make launch and restart lifecycle deterministic - #2313

Merged
BigPizzaV3 merged 3 commits into
BigPizzaV3:mainfrom
Yuimi-chaya:codex/restart-logic-20260926
Oct 2, 2026
Merged

BigPizzaV3 merged 3 commits into
BigPizzaV3:mainfrom
Yuimi-chaya:codex/restart-logic-20260926

Conversation

@Yuimi-chaya

@Yuimi-chaya Yuimi-chaya commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

概要

修复管理器的直接启动/重启生命周期与并发竞态。基于 main 的 ab203409,与 #2309 的注入健康检测保持独立。

生命周期

  • 启动与重启使用后台阻塞任务,并共用 RAII 单飞锁;顶栏、概览、Dream Skin 和维护页面的四个按钮共用等待状态。
  • Codex 未运行且未要求同步供应商时,重启按直接启动处理;显式供应商同步保留完整重启流程。存在性依据原生进程,不依据 CDP 是否可达。macOS 按调试端口定位,Windows 等其他平台按 Codex 桌面进程定位。
  • 供应商同步生命周期锁覆盖停止与配置同步,在启动新实例前验证释放;不能释放则不启动,消除先等空闲、再停止之间的 TOCTOU 窗口。
  • 严格执行确认 Codex 退出 → 等待原生清理 → 确认旧启动器退出 → 启动新实例;进程未退出、清理仍在进行等阻断错误不执行后续步骤。
  • 保留 NativeBrowserShutdown:清理仍在进行或回执无效时中止;已完成但恢复失败时保留 RestoreFailed、诊断与前端告警,按上游既有语义继续后续启动。
  • 完成提示核对本次请求时间,不把旧成功/失败快照或仍在启动的状态当作本次结果。

恢复校验

采用上游 recovery_material(paths, key),删除已失效的恢复校验契约参数与 _with_contract 等中转接口。不回退 04ab629 的未知运行时适配或已合入的浏览器诊断。

取得 monitor 锁后,使用上游恢复材料验证、控制状态与实际服务字节作只读核验。不创建新清理锁,不覆盖外部修改,不用旧版固定指纹阻止新运行时恢复。

有意放宽的范围:monitor 已释放锁、回执为合法已知状态,即使回执仍为 active 或 blocked,只要控制状态和磁盘内容证明恢复完成,就可以返回 Ready。这解决已恢复磁盘上的陈旧回执阻断启动的问题。

未放宽的范围:active 或 restored 回执但磁盘内容不匹配仍报错;无效/未知回执、仍持锁的 monitor 均不放行。blocked 且磁盘未恢复仍返回上游的 RestoreFailed,不冒充 Ready。回归测试直接核对服务和控制文件未被改写。

另外修复上游绑定自检的一处路径误判:新增 JSON 控制路径中的 nf/ze/cD 子串不是源码标识符漂移。保留原计数检查,只补偿新增路径字面量的出现次数,并用 C:/conflict/nf-ze-cD/control.json 作确定性回归。不改变协议、组件检查或恢复校验。

验证

  • cargo test --locked --offline -j 2 -p codex-plus-core -p codex-plus-manager --no-fail-fast:1512 passed,0 failed,6 ignored;其中 core 1413、manager 74、Windows subsystem 25。
  • 前端测试:332 passed,0 failed;TypeScript 与 Vite 生产构建通过。
  • PR diff 检查通过。移除启动流程的源码字符串位置断言,保留纯函数 disposition、单飞、锁释放、停止顺序和失败短路测试。

以上是本次 rebase 后的 Windows 本地验证。忽略项未计为通过;未运行真实应用重启、安装或替换,未做 macOS 本地验收,未生成新的 setup/ZIP。保留上游既有警告;之前的真人测试不代表当前 rebased 提交已重新通过真人验收。

Yuimi-chaya added a commit to Yuimi-chaya/CodexPlusPlus that referenced this pull request Sep 28, 2026
@BigPizzaV3

Copy link
Copy Markdown
Owner

审查了。改动方向认可(启动/重启生命周期确实需要确定性),但 rebase 到当前 main 会遇到 9 处语义冲突,需要你确认后再合。

冲突点

git rebase origin/main 在三个文件上冲突:

crates/codex-plus-core/src/native_browser.rs         3 处
apps/codex-plus-manager/src-tauri/src/commands.rs    5 处
apps/codex-plus-manager/src/App.tsx                  1 处

其中 native_browser.rs 不是机械冲突 —— main 与你的分支各自独立演进了同一块代码:

函数 main 你的分支
verify_restored_state (paths) (paths, contract)
wait_for_monitor_shutdown_at 返回 Result<NativeBrowserShutdown>,无 _with_contract 变体 保留该签名,另加 wait_for_monitor_shutdown_with_contract 返回 Result<()>

main 上 NativeBrowserShutdown 有 9 处引用、RuntimeContract 27 处,说明两边都做了重构但落点不同。直接取一侧会丢掉另一侧的语义(特别是 NativeBrowserShutdown 的返回值,commands.rs 那 5 处冲突很可能也依赖它)。

你的分支里已经有一次 a2bc72b4 Merge upstream/main to resolve PR #2313 conflicts,但此后 main 又推进了 —— 尤其刚合入的 #2311 和 #2335 都动了 native_browser.rs。

为什么请你 rebase 而不是我代解

这 9 处里有多个是「保留哪一边的返回语义」的设计选择,而不是取并集就对的文本冲突。我对你的设计意图只能反推,代解很可能静默丢掉一边的行为 —— 这个仓库里已经发生过一次「基于旧基线静默回退修复」(#2202 回退了 AUMID 读取),不希望再添一例。

建议

rebase 到当前 main,重点确认:

  1. wait_for_monitor_shutdown 的返回类型以哪边为准(NativeBrowserShutdown 还是 Result<()>)
  2. verify_restored_state 是否需要 contract 参数 —— 若需要,main 上另外两处调用点要一并更新
  3. commands.rs 那 5 处冲突是否依赖上述签名

改完请跑 cargo test -p codex-plus-core --no-fail-fast(当前 main 是 1432 passed / 0 failed)与前端测试,确认无回归。

边界确认

你在正文里写明了与 #2311、#2335 的边界(浏览器状态与 CUA 适配不在本 PR 内),这个划分是对的,两者我也已按各自范围处理,不冲突。

@Yuimi-chaya
Yuimi-chaya force-pushed the codex/restart-logic-20260926 branch from a2bc72b to 1236496 Compare September 30, 2026 08:09
@Yuimi-chaya

Copy link
Copy Markdown
Contributor Author

感谢指出这些语义冲突,已按这个方向处理并 rebase 到当前 main 的 3d6d360,更新后的功能提交为 1236496。原来的 merge commit 已从 PR 提交序列中移除,相对 main 仍只修改原来的六个文件。

逐项确认如下:

  1. 返回类型以上游 NativeBrowserShutdown 为准。 公共 wait_for_monitor_shutdown、内部 _with_contract 和测试入口 _at 都返回 Result<NativeBrowserShutdown>,不会把 RestoreFailed 降成 Result<()> 或直接当成功忽略。
  2. 保留 verify_restored_state(paths, contract),两个调用点均已更新。 参数用于只读恢复校验和测试契约注入,生产仍沿用已验证的契约;上游 recovery_material 对旧版和新版原始哈希的支持完整保留,不回退 fix(browser): [紧急兼容] 适配新版 Windows CUA 纯 API 浏览器调用 #2335。取得 monitor 锁后,已知陈旧回执必须通过控制状态与磁盘内容校验才算 Ready;blocked 且未恢复仍是 RestoreFailed,不覆盖外部修改。无效或未知回执、仍持锁的 monitor,以及声称恢复但磁盘不匹配,仍报错阻止新实例启动。
  3. 管理器和前端完整保留枚举语义。 停止顺序仍是确认 Codex 退出 → 等待原生清理 → 确认旧启动器退出;RestoreFailed 保留诊断日志、nativeBrowserRestoreFailed 字段及前端“恢复失败,仍会继续启动”告警。上游的固定 helper 端口等待、fix(browser): 分离原生浏览器可用状态与兼容补丁诊断 #2311 浏览器诊断和 fix(browser): [紧急兼容] 适配新版 Windows CUA 纯 API 浏览器调用 #2335 CUA 适配也保留,没有整文件取某一侧。

新增/强化的回归测试覆盖陈旧回执校验、RestoreFailed 返回值、外部文件与控制状态保持不变,以及已恢复磁盘上的未知回执拒绝。

rebase 后本地验证:

  • cargo test --locked -j 2 -p codex-plus-core --no-fail-fast:1375 passed / 0 failed / 6 ignored。
  • cargo test --locked -j 2 -p codex-plus-manager --no-fail-fast:75 passed,Windows subsystem 22 passed,0 failed。
  • 前端测试:275 passed / 0 failed;TypeScript、Vite 生产构建和 PR 相对 main 的 git diff --check 均通过。

core 的忽略项保留其私有运行时、真实浏览器或 E2E 环境要求,未计为通过;这轮没有关闭/重启实际应用、替换安装文件或做 macOS 本地验收。PR 正文已同步上述语义、范围和验证结果,请再审查。

@BigPizzaV3

Copy link
Copy Markdown
Owner

这个 PR 可以合并——方向正确、方案质量高。当前冲突是陈旧基线,不是方案缺陷。请 rebase 后即可合入。

我在临时 worktree 里按最保守方式解了冲突(native_browser.rs 取 main 侧),整棵树编译并全绿:

cargo check -p codex-plus-core --lib     通过(仅 main 上既有的 dead-code warning)
cargo test -p codex-plus-core            1395 passed / 0 failed
cargo test -p codex-plus-manager --lib   75 passed / 0 failed
npm test                                 331 passed / 0 failed
npm run check + vite:build               通过

冲突只有 1 处,且不涉及取舍

// main (HEAD)
let (_, original, _) = recovery_material(paths, &key)?;
// 本 PR
let (_, original, _) = recovery_material(paths, &key, contract)?;

根因是 main 的 04ab6290(issue #2294,CUA 白名单改认结构不认哈希)把 recovery_material 的签名从 3 参改成 2 参:

-fn recovery_material(paths, key, contract: &RuntimeContract) -> ...
+fn recovery_material(paths: &BrowserPaths, key: &str) -> ...

新实现的校验比旧版更强(native_browser.rs:772-790):journal.schema == 1 && sha(&original) == journal.original_sha && journal.candidate_sha == sha(&candidate) && journal.modified_nanos < 1e9。所以 PR 传 contract 的目的(verify_restored_state 里的 RuntimeContract::pinned())已被 main 的改动整体取代——contract 这个参数在新设计下没有可做的事。

取 main 侧即可,不丢任何语义。

必须删掉的幽灵参数

rebase 时请一并处理:verify_restored_state(paths, contract)(native_browser.rs:1033)与 wait_for_monitor_shutdown_with_contract(paths, timeout, contract)(native_browser.rs:1131)保留后,contract 变成零使用的参数 → unused variable: contract warning。

PR 描述里「保留 verify_restored_state(paths, contract) 用于只读校验和测试契约注入」这个理由,在 04ab6290 之后已经不成立了。

如果目标是保留「用契约做只读校验」的能力,那是另一件事,需要先论证 pinned() 的硬编码哈希表还要不要——而 main 的方向恰恰是去掉它(issue #2294 的原话:「一次版本即作废整张表」)。请不要把 #2294 回退掉。

方案质量:确实消除了竞态

值得肯定,四处都是结构性修复而非加延时:

  1. 单飞保护是真锁:try_acquire_launch_guard() 用 AtomicBool::compare_exchange,LaunchGuard 的 Drop 复位,launch 与 restart 共用。
  2. 同步锁改为全生命周期持有 —— 这是本 PR 最有价值的一处。旧实现是「轮询等空闲 → 然后强杀 launcher」(issue [Bug]: 连续重启会强杀正在执行 provider sync 的 launcher,留下死亡 PID 锁并导致会话消失 #1901),是典型 TOCTOU:等空闲与强杀之间存在窗口,新同步可能恰好拿到锁然后被杀,留下 ~/.codex/tmp/provider-sync.lock 残留。改成真持有锁并在新实例启动前释放,才是「deterministic」的实质。release() 的 ABA 检测保住了,且释放失败会阻止启动(有测试覆盖)。
  3. 进程退出改为事后核实:stop_codex_processes_for_debug_port_and_wait 超时后原本只是放弃等待,PR 补了 anyhow::ensure!(...is_empty(), "Codex 进程尚未退出;未启动新实例")——把「等不到就当没事」变成「等不到就中止」。
  4. 停止顺序短路:RestartStopError::Codex 让「Codex 未确认退出」时不清理原生浏览器、不停旧 launcher,测试用 panic!("must not wait on cleanup") 守卫,写法干净。

一处提醒:native_browser.rs:1163-1180 的回执校验里,restored 回执但磁盘校验失败时本 PR 返回 Ready(继续启动),而 main 会 bail!。理由(已释放锁的已知陈旧回执 + 不覆盖外部修改)站得住,但属于放宽而非收紧,建议在 PR 描述里明确标注为「有意放宽」,否则将来会被当成回归。

次要建议(不阻断)

  1. restart_as_launch_precedes_cleanup_and_does_not_use_cdp_presence(commands.rs:6876-6887)是源码字符串断言(include_str! + 比位置),重构即失灵且抓不到语义。同样的意图用 restart_disposition 的纯函数测试就能表达,建议删掉或降级为注释。
  2. commands.rs:7331 / 7994 附近把 /// 文档注释移到了 #[test] 之上。两者都能编译,但与仓库其余部分风格不一致,属 hunk 重排噪声,rebase 时顺手还原。
  3. find_codex_processes_for_debug_port 在非 macOS 上忽略 debug_port、返回所有 Codex 桌面进程(watcher.rs:676-679),所以 LaunchOnly 分支在 Windows 上几乎不可达。不会出错(停止侧同样退化成全量),但命名有误导性,建议加平台差异注释。
  4. 前端防重复点击只覆盖了 3 个入口中的 1 个:只有 App.tsx:3651(顶栏「重启 Codex++」)加了 disabled={launchPending},而 App.tsx:5254(Dream Skin「重启并应用」)与 4560 / 6824(「启动 Codex++」)没有。后端单飞保护会兜住,但 PR 描述里「前端按钮在等待启动结果期间禁用」字面上只兑现了三分之一,改动很小建议补齐。
  5. 344 行压在单个 commit(1236496)里,建议拆成 2-3 个(真锁替换轮询 / 停止流程事后核实 / 前端单飞与完成判定),将来 bisect 会容易很多。

测试有效性

有效部分是关键的:launchCompletionNotice 覆盖了 null / starting / 过期快照三类必须落到 accepted 的情形——这修掉了一个真实缺陷(旧实现没比对 requestedAt,一次失败的重启之后紧接着点启动,前一次的 failed 快照会被当成本次结果)。三个 native_browser 新测试都断言了没被改写(比对 control.json 与 service 的字节),正是这类改动最该测的。

另外 PR 描述里的验证数字已过期(写 275,实测 331),合并前值得重跑一次。

请 rebase 到当前 main(ea82bc74 之后),按上面第 1 点解冲突、删掉幽灵参数,重跑测试即可。不需要改设计。

BigPizzaV3 added a commit that referenced this pull request Oct 2, 2026
审计当时的处置建议有两处被实测推翻、一处性质被更正,记下来:

- **推翻 #2303 的「并发验证阻断项」**:默认并发 5 次全过(未复现 PR 描述的失败);
  32 线程加压下 main 与 PR 分支会挂**同一个**用例
  (upstream_request_returns_when_provider_accepts_but_never_sends_headers,
  墙钟断言 assert!(started.elapsed() < 1s))。是既有 flake,不是 PR 引入。
  真正阻断项是分支落后 34 提交。
  另更正「保留 compaction: bool 单一标记路线」这条约束——加入原生透传后
  compaction 语义已分裂,多字段反而更清晰。
- **推翻 #2313 的「范围过大需拆分」**:冲突只有 1 处且是陈旧基线
  (main 的 04ab629 把 recovery_material 改成 2 参),解完整树全绿 1395/75/331。
  不该因冲突要求改设计。
- **补充 #2333 的真实回归**(审计未发现):实测 model_catalog 测试红,
  默认模型被静默换成 modelList 第一条。附教训:审查贡献者 PR 时必须单独验
  「PR 自带测试是否覆盖整个 workspace」,只信 PR 描述的测试命令会漏跨 target 回归。
- 补记 #2309 两个阻断项的实测证据(版本号撞车、Err(_) 抵消退避修复)。

执行结果:#2366 已合入(ea82bc74);#2337 / #2371 由已落地提交覆盖后关闭;
#2333 / #2309 回复要求修改;#2313 / #2303 回复可合但需先 rebase。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Yuimi-chaya
Yuimi-chaya force-pushed the codex/restart-logic-20260926 branch from 1236496 to 540b862 Compare October 2, 2026 13:52
@Yuimi-chaya

Copy link
Copy Markdown
Contributor Author

已 rebase 到 main 的 ab203409,当前 HEAD 为 540b8628,相对 main 仍是原来的六个文件。

按评论处理:

  1. 冲突采用上游 recovery_material(paths, key);已删除 verify_restored_state 与 shutdown 等待链里的失效 contract 参数及 _with_contract 中转,不回退 04ab629。
  2. 四个启动/重启按钮均已接入共享 launchPending,跨页面组件通过显式 props 传递。保留后端 RAII 单飞锁。
  3. 删除启动流程源码字符串位置断言,保留纯函数 disposition 测试;清理测试属性噪声,并补充 macOS/其他平台进程定位差异注释。更新分为三个提交,便于追踪本次适配与后续小修正。
  4. 正文明确标注陈旧回执处理的有意放宽:只有合法已知回执已解锁、控制状态及磁盘恢复核验通过,才允许 Ready。

补充一点:我核对当前分支后,restored 回执但磁盘核验失败仍然报错,并不是返回 Ready。现在把 active/restored 回执配合外部文件修改的拒绝分支都加入同一回归,直接断言服务与控制文件不被改写。blocked 且未恢复保持上游 RestoreFailed。

重跑测试发现上游计数自检还会误把新增控制文件路径里的压缩标识符子串当作源码变化,例如本机 D 盘 conflict 路径中的 nf。已作最小补偿,仅计入新增 JSON 路径字面量的次数,保留其余检查,并增加 nf-ze-cD 路径回归;没有带入新的 CUA 适配方案。

本地结果:core 1413 + manager 74 + Windows subsystem 25,共 1512 passed/0 failed/6 ignored;前端 332 passed,TypeScript、Vite 和 diff 检查通过。忽略项未计为通过;未执行真实应用重启、安装替换或 macOS 本地验收。PR 正文已更新。

BigPizzaV3 added a commit that referenced this pull request Oct 2, 2026
当晚新提的 PR(@Yuimi-chaya,+1370/-16,11 文件),与已合入的 04ab629 改同一块
native_browser.rs。处置:要求修改。

三条经独立核实的阻断项:
1. FileCheck::Structural 降级通道被整体旁路——for_manifest 改收哈希字符串,
   未知版本必然 Err,main 的结构降级分支在未知版本上不可达。
2. ENTRY_SHA 重新引入哈希白名单(无条件准入 cua-repl.mjs 入口),
   复现 issue #2294 的病根;PROTOCOL_SHAPES 同理。
3. 运行时版本下限检查被静默弱化。

确属增量应保留:组件事务守卫、schema-2 可重算恢复、绑定级 AST 校验。
依赖合规(@babel 零膨胀,188→188),但构建 --check 未接 CI、esbuild 未显式声明。
实测 1383 passed / 315 pass,但最有价值的负例全在被 skip 的 fixture 测试里,
detect 路径在 CI 上零覆盖。

安全:子进程隔离与写盘原子性落实,未触碰身份认证(已独立确认)。
合并顺序:与 #2309/#2313 三者都不含 04ab629,必须先 rebase。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@BigPizzaV3
BigPizzaV3 merged commit c509c06 into BigPizzaV3:main Oct 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants