Skip to content

fix(acp): detect CLI config errors on startup and provide actionable guidance#1947

Merged
piorpua merged 1 commit intomainfrom
fix/issue-1143
Mar 31, 2026
Merged

fix(acp): detect CLI config errors on startup and provide actionable guidance#1947
piorpua merged 1 commit intomainfrom
fix/issue-1143

Conversation

@kaizhou-lab
Copy link
Copy Markdown
Collaborator

Summary

  • When a CLI (e.g. Codex) fails to start due to a config file parsing error, detect the error loading config pattern in stderr and provide an actionable message with the config file path
  • Extract the startup error classification logic into a standalone buildStartupErrorMessage() function for testability
  • Add comprehensive unit tests covering all startup error patterns (config error, command not found, generic error)

Related Issues

Closes #1143

Test Plan

  • Unit tests pass (8 new tests in acpStartupError.test.ts)
  • Type check passes
  • Lint and format pass

@piorpua piorpua added the bot:reviewing Review in progress (mutex) label Mar 30, 2026
@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 30, 2026

Code Review:fix(acp): detect CLI config errors on startup and provide actionable guidance (#1947)

变更概述

本 PR 将 AcpConnection.setupChildProcessHandlers 中的启动错误消息构建逻辑提取为独立的纯函数 buildStartupErrorMessage(),并新增了 CLI 配置文件加载错误的检测模式。同时添加了 8 个单元测试覆盖所有分支路径。涉及 src/process/agent/acp/AcpConnection.ts 和新增测试文件 tests/unit/acpStartupError.test.ts


方案评估

结论:✅ 方案合理

将内联的错误消息构建逻辑提取为纯函数是正确的重构方向——参数显式传入,无副作用,完全可测试。新增的 config error 检测模式针对 Codex config.toml 解析失败的实际场景,提供了可操作的用户指引。检测链的顺序设计合理(config error 后于 command-not-found 执行,优先级更高),与测试用例一致。


问题清单

未发现 CRITICAL、HIGH、MEDIUM 级别问题。


汇总

# 严重级别 文件 问题
未发现问题

结论

  • 批准合并 — 无阻塞性问题

纯函数提取干净,测试覆盖充分(8 个用例覆盖所有分支),与项目架构一致,代码质量良好。


本报告由本地 pr-review skill 生成,包含完整项目上下文,无截断限制。

CONCLUSION: APPROVED
IS_CRITICAL_PATH: false
PR_NUMBER: 1947

@piorpua piorpua added bot:needs-human-review Blocking issues — human must intervene and removed bot:reviewing Review in progress (mutex) labels Mar 30, 2026
…guidance

When a CLI (e.g. Codex) fails to start due to a config file parsing error,
extract the config file path from stderr and show a clear message suggesting
the user review or temporarily rename the problematic config file.

This addresses cases like Codex multi-agent config.toml settings that the
ACP bridge cannot parse (invalid type errors).
@piorpua piorpua added bot:reviewing Review in progress (mutex) and removed bot:needs-human-review Blocking issues — human must intervene labels Mar 31, 2026
@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 31, 2026

✅ 已自动 review,无阻塞性问题,正在触发自动合并。

@piorpua piorpua merged commit dd6f96d into main Mar 31, 2026
17 checks passed
@piorpua piorpua deleted the fix/issue-1143 branch March 31, 2026 09:05
@piorpua piorpua added bot:done Auto-merged by bot and removed bot:reviewing Review in progress (mutex) labels Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:done Auto-merged by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 不兼容codex多Agent模式

2 participants