Skip to content

fix(codex): 保留原生 fork 失败原因与清理状态 - #4018

Merged
MagicLizi merged 3 commits into
mainfrom
dash/codex-fork-recovery
Sep 7, 2026
Merged

fix(codex): 保留原生 fork 失败原因与清理状态#4018
MagicLizi merged 3 commits into
mainfrom
dash/codex-fork-recovery

Conversation

@dashhuang

@dashhuang dashhuang commented Sep 6, 2026

Copy link
Copy Markdown
Member

这次改了什么

摘要

Codex 一次性 fork 在操作失败后仍会清理子线程。如果清理同时失败,原来的 finally 会覆盖最初异常,调用方失去真正的失败原因。本 PR 保留操作异常及其阶段,清理失败只增加诊断标记;历史恢复、认证、源线程准备受阻和取消信号保持原有身份。

变更类型

  • feat 新功能
  • fix 缺陷修复
  • refactor / perf 重构或性能优化
  • docs / test / chore 文档、测试或工程维护
  • 其他:

范围

UI 变化

不涉及。

  • 引用的设计规范:不涉及:仅修改 maker-core 原生 fork 错误处理与测试。

怎么验证的

自动验证

pnpm --filter @cindy/maker-core exec vitest run src/agents/codex/index.test.ts -t CodexAgent.forkSdkSession
结果:27 项 fork 测试通过。

pnpm test:unit(经统一 run-unit-gate.sh 执行)
结果:通过,GATE_EXIT=0。

pnpm --filter desktop run --if-present typecheck
pnpm --filter @cindy/maker-core run --if-present typecheck
结果:Desktop 通过;maker-core 无 typecheck script,按 --if-present 跳过。

git diff --check
结果:通过。

覆盖启动失败后 host 清理、启动与退出同时失败、fork 响应丢失、rollback 与子线程清理同时失败,以及准备受阻和取消信号身份。保留既有正常 fork、索引历史保护与共享 host 行为测试。

手工验证

本轮未启动真实 Desktop 或修改用户原生历史;本轮以同步后的代码和自动测试验证集成。

未执行的验证

未进行真实双账号 Desktop UI 切换或 Windows 实机验证。先前旧版跨来源 fork 的模拟重试实验不作为本次最终实现的验证依据;最新提交由 CI 继续验证 Linux/Windows。

风险

风险分类

  • 无已知风险
  • SQLite / migration
  • system prompt
  • 协议兼容
  • 权限 / 安全 / 用户数据
  • 存量插件兼容(批准状态 / 指纹 / manifest 校验 / 安装布局 / 包格式)
  • 原生层 / fingerprint / OTA
  • 跨平台差异
  • 其他:原生 fork 的异常出口与清理顺序。

影响与回滚

  • 影响范围:本地一次性 Codex fork;原生锚点共享 host 快捷路径、源历史保护和模型选择时机保持不变。原始 cause 只留在进程内,不新增 IPC 字段或日志上传内容。
  • 回滚 / 降级方式:回退本 PR 即恢复原有异常处理,无持久状态或数据迁移。不同平台的进程关闭失败仍需 CI/实机覆盖。

提交前检查

  • 已 review 完整 diff
  • 每个 commit 都带 DCO 签名(git commit -s,见 DCO
  • UI 改动已在「UI 变化」注明引用的设计规范章节(不涉及 UI 则跳过)
  • 未提交凭证、令牌或授权文件
  • 已补充必要文档(代码注释与本 PR 验证边界)
  • 已确认测试结果或说明未执行原因

@dashhuang
dashhuang requested a review from a team as a code owner September 6, 2026 22:39
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T00:21:46.769453Z 8c7eda6 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown

Greptile Summary

本 PR 为 Codex 跨来源线程重建增加分阶段错误包装、有限安全重试、清理失败保留及脱敏 IPC 诊断。

  • 仅在 host-create/host-start 的明确瞬时连接故障后重试一次。
  • thread/fork 发出后的失败不会重放,避免重复创建线程。
  • 清理异常不覆盖原始操作异常,并通过现有 CAS 控制最终路由提交。
  • 新增覆盖重试、恢复信号、清理失败、并发提交和诊断脱敏的回归测试。

Confidence Score: 4/5

此 PR 总体可合并,但建议修正 401/403 裸数字匹配,以免部分本地代理连接故障绕过预期的安全重试。

分阶段包装、清理和 CAS 路径未发现阻塞性问题;唯一确认的问题是认证排除正则可能将端口 401/403 误判为 HTTP 认证失败,导致一次可安全恢复的连接故障不被重试。

Files Needing Attention: packages/maker-core/src/agents/codex/fork-error.ts

Important Files Changed

Filename Overview
packages/maker-core/src/agents/codex/fork-error.ts 新增 fork 阶段及瞬时连接错误分类;裸 401/403 文本匹配可能把端口号误判为认证状态。
packages/maker-core/src/agents/codex/index.ts 为 fork 生命周期记录失败阶段,保留控制信号和首个异常,并在 finally 中清理子线程及隔离 host。
apps/desktop/src/main/maker-ipc/codexProviderThreadRelink.ts 增加最多两次的安全 fork 尝试、尝试次数包装及脱敏失败描述。
apps/desktop/src/main/maker-ipc/register.ts 将有限重试接入 provider relink,并按失败阶段记录日志和生成有界 IPC 诊断。
packages/maker-core/src/agents/codex/fork-error.test.ts 覆盖连接码、认证/取消排除和循环 cause,但未覆盖网络错误文本中的 401/403 端口号。

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[准备源线程] --> B[创建隔离 host]
  B --> C[启动 host]
  C -->|明确瞬时连接错误且清理成功| D[等待 1 秒]
  D -->|仅一次| B
  C --> E[发送 thread/fork]
  E -->|失败| F[禁止自动重放]
  E -->|成功| G[预留替代线程清理]
  G --> H{路由 CAS 成功?}
  H -->|是| I[提交目标 provider 路由]
  H -->|否| J[清理替代线程并保留旧路由]
Loading
Prompt To Fix All With AI
### Issue 1
packages/maker-core/src/agents/codex/fork-error.ts:54-58
**端口号被误判为认证状态**

`transportCode` 会把错误文本中任何独立的 `401``403` 都当成认证失败。例如,本地代理报出 `connect ECONNREFUSED 127.0.0.1:401` 时,这里的检查会先命中端口号并返回 `null`,导致本应识别为瞬时连接错误的 `ECONNREFUSED` 不会触发这次新增的安全重试。建议只将结构化 HTTP 状态,或带有明确 HTTP/认证语义的 401、403 文本视为认证失败。

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(codex): 保留跨来源切换异常并限制安全重试" | Re-trigger Greptile

Comment thread packages/maker-core/src/agents/codex/fork-error.ts Outdated
@MagicLizi MagicLizi added status:ci-running CI 还在跑(review-pr 自动维护,仅展示) touches:core 改动碰到架构核心路径(review-pr 自动维护,仅展示) labels Sep 6, 2026
@MagicLizi MagicLizi removed the status:ci-running CI 还在跑(review-pr 自动维护,仅展示) label Sep 6, 2026
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>

@MagicLizi MagicLizi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代码重审通过:当前 head 未发现 P0/P1,全部 conversation 已 resolve。跨来源 fork 失败会保留原异常,仅瞬时 host-start 安全重试一次。#4011 合入后本 PR 与 main 有冲突,本轮先 Approve,合并等冲突解开。

@MagicLizi

Copy link
Copy Markdown
Contributor

@dashhuang 👋 这个 PR 目前与 main 有合并冲突,auto-review 因此暂时跳过、没法继续审查 / 合并。

请在本地 merge 最新的 origin/main 解决冲突后推送;冲突解除后,下一轮 auto-review 会自动重新处理这个 PR。

@MagicLizi MagicLizi added the status:conflict 与目标分支有冲突(review-pr 自动维护,仅展示) label Sep 7, 2026
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
@dashhuang
dashhuang force-pushed the dash/codex-fork-recovery branch from 86c089c to 8c7eda6 Compare September 7, 2026 00:17
@dashhuang dashhuang changed the title fix(codex): 保留跨来源切换异常并限制安全重试 fix(codex): 保留原生 fork 失败原因与清理状态 Sep 7, 2026
@MagicLizi MagicLizi added status:awaiting-bot-review 等外部审查机器人表态(review-pr 自动维护,仅展示) and removed status:conflict 与目标分支有冲突(review-pr 自动维护,仅展示) touches:core 改动碰到架构核心路径(review-pr 自动维护,仅展示) status:awaiting-bot-review 等外部审查机器人表态(review-pr 自动维护,仅展示) labels Sep 7, 2026

@MagicLizi MagicLizi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

独立审查通过(standard):Codex 隔离 fork 失败会带上 stage/cause,cleanup 失败不再盖掉原错误。无 P0/P1。

@MagicLizi
MagicLizi merged commit dacc911 into main Sep 7, 2026
20 checks passed
@MagicLizi
MagicLizi deleted the dash/codex-fork-recovery branch September 7, 2026 01:28
@MagicLizi

Copy link
Copy Markdown
Contributor

合了。隔离 fork 失败以后还能看见是哪一步挂的、清理有没有盖掉原因——排 Codex 原生 fork 问题会少绕一圈。

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.

3 participants