Codecov Report

❌ Patch coverage is 58.47458% with 49 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

感谢提交！在 Linux 上修复 ACP optional dependency 问题的思路很好，fallback 策略覆盖了缓存查找、平台直装和 meta 包回退，考虑得很全面。

提两个小建议：

1. 非 Windows 平台的包建议也锁定版本号

目前 resolveCodexAcpPlatformPackageSpecifier 只在 Windows 上拼接版本号：

function resolveCodexAcpPlatformPackageSpecifier(packageName: string): string {
  return process.platform === 'win32' ? `${packageName}@${CODEX_ACP_BRIDGE_VERSION}` : packageName;
}

这样 Linux/macOS 上 npx 会安装 latest 版本，如果上游发了不兼容的新版就会出问题。建议统一锁定：

function resolveCodexAcpPlatformPackageSpecifier(packageName: string): string {
  return `${packageName}@${CODEX_ACP_BRIDGE_VERSION}`;
}

2. resolveCodexAcpPlatformPackage 可以去掉

connectCodex 已经有完整的 fallback 机制（候选包逐个尝试，失败自动试下一个），所以不需要预先校验平台是否支持。不支持的平台 npx 安装失败后自然会回退到 meta 包。

直接拼字符串就够了：

const platformPackage = `@zed-industries/codex-acp-${process.platform}-${process.arch}@${CODEX_ACP_BRIDGE_VERSION}`;

这样 resolveCodexAcpPlatformPackage、resolvePreferredCodexAcpPlatformPackage、shouldPreferDirectCodexAcpPackage 三个函数和相关的 null 检查都可以删掉，逻辑会简洁很多。

感谢提交！在 Linux 上修复 ACP optional dependency 问题的思路很好，fallback 策略覆盖了缓存查找、平台直装和 meta 包回退，考虑得很全面。

提两个小建议：

1. 非 Windows 平台的包建议也锁定版本号

目前 resolveCodexAcpPlatformPackageSpecifier 只在 Windows 上拼接版本号：

function resolveCodexAcpPlatformPackageSpecifier(packageName: string): string {
  return process.platform === 'win32' ? `${packageName}@${CODEX_ACP_BRIDGE_VERSION}` : packageName;
}

这样 Linux/macOS 上 npx 会安装 latest 版本，如果上游发了不兼容的新版就会出问题。建议统一锁定：

function resolveCodexAcpPlatformPackageSpecifier(packageName: string): string {
  return `${packageName}@${CODEX_ACP_BRIDGE_VERSION}`;
}

2. resolveCodexAcpPlatformPackage 可以去掉

connectCodex 已经有完整的 fallback 机制（候选包逐个尝试，失败自动试下一个），所以不需要预先校验平台是否支持。不支持的平台 npx 安装失败后自然会回退到 meta 包。

直接拼字符串就够了：

const platformPackage = `@zed-industries/codex-acp-${process.platform}-${process.arch}@${CODEX_ACP_BRIDGE_VERSION}`;

这样 resolveCodexAcpPlatformPackage、resolvePreferredCodexAcpPlatformPackage、shouldPreferDirectCodexAcpPackage 三个函数和相关的 null 检查都可以删掉，逻辑会简洁很多。

gh 确实装好了，版本是 2.87.3。我已经把 PR #1594 的评论和你本地分支都对过了；当前本地和远端都还停在 88d80b9，也就是这些建议还没有被后续 commit 处理。

我的判断是：

• getNpxCacheDir() 这条评论是成立的。现在的实现会在 POSIX 上优先选“先存在的父目录”而不是“真正存在 _npx 的目录”，代码在 shellEnv.ts#L549。这不只影响缓存命中，也会影响损坏缓存清理；AcpConnection 和 McpProtocol 都在复用它。对应测试 getNpxCacheDir.test.ts#L68 现在也把这个偏弱的行为写死了，所以这条我建议采纳。

• “POSIX 平台包统一锁到 CODEX_ACP_BRIDGE_VERSION” 这条不能直接照做。你项目里 bridge 版本固定在 acpTypes.ts#L38 的 0.9.5，但我直接查了 npm registry：
  @zed-industries/codex-acp-linux-x64 最新只有 0.9.2，
  @zed-industries/codex-acp-linux-arm64 最新也是 0.9.2，
  @zed-industries/codex-acp-darwin-x64 最新 0.9.4，
  @zed-industries/codex-acp-darwin-arm64 才是 0.9.5。
  我还验证了 npm view @zed-industries/[email protected]，结果是 404。所以 reviewer 的“版本漂移风险”判断有道理，但“统一锁成 0.9.5” 现在会直接把 Linux 和部分 macOS 打挂。

• resolveCodexAcpPlatformPackage() 那条“可以删掉、直接拼字符串”的建议，属于可选简化，不是必须修。单独看它问题不大，但它和上面那条“统一锁版本”绑在一起就不安全了。当前显式映射虽然啰嗦一点，但至少更可控，代码在 acpConnectors.ts#L73 一带。

• “cached binary 要校验版本，不能直接按 mtime 取最新” 这个风险也是真实的，代码在 acpConnectors.ts#L377。但 review 里建议它必须匹配 CODEX_ACP_BRIDGE_VERSION，这在 POSIX 上同样会撞上平台包版本不齐的问题，所以也不能原样照搬。这个点如果要修，我更倾向于单独设计一个安全方案，而不是直接套评论里的写法。

我建议你现在的处理顺序是：先修 _npx 目录探测这条；另外两条和“统一锁版本”相关的 comment 先不要直接改，可以回评论说明 npm registry 目前平台包版本和 meta 包不齐平。你要的话，我下一步可以直接帮你做一个安全 follow-up patch，只改 _npx 探测和对应测试，再顺手帮你起一段回复 review comment 的文案。

Code Review：fix(acp): improve packaged codex startup fallback (#1594)

变更概述

本 PR 修复了 Linux 环境下的 Codex ACP 启动失败问题，主要新增了三项机制：(1) 通过扫描 npx 缓存目录直接定位已缓存的平台原生二进制文件；(2) 构建多候选包的顺序重试循环，根据平台优先直接包或元包；(3) 将 Codex 的连接超时从固定 70 秒提升至 150 秒，适配 Linux 上较慢的启动速度。此外同步增强了 getNpxCacheDir 以识别 npm_config_cache 环境变量和 ~/.npm-cache 路径。

方案评估

结论：✅ 方案合理

方案通过缓存二进制发现 + 多候选回退的两层机制解决了 Linux 上平台包安装不可靠的根本问题，与项目 ACP 架构一致。回退链路清晰，每次失败后调用 cleanup 再尝试下一个候选，逻辑正确。测试覆盖了 Windows、Linux、macOS 三平台的缓存命中和多级回退路径，质量较高。

问题清单

🟡 MEDIUM — Linux/macOS 平台包缺少版本固定

文件：src/process/agent/acp/acpConnectors.ts，第 73–75 行

问题代码：

function resolveCodexAcpPlatformPackageSpecifier(packageName: string): string {
  return process.platform === 'win32' ? `${packageName}@${CODEX_ACP_BRIDGE_VERSION}` : packageName;
}

问题说明：Windows 平台固定了版本（如 @zed-industries/[email protected]），但 Linux 和 macOS 直接使用不带版本的包名（如 @zed-industries/codex-acp-linux-x64），npm 会解析为 latest。若平台包发布了与当前协议版本（CODEX_ACP_BRIDGE_VERSION=0.9.5）不兼容的新版本，则可能出现协议不匹配故障。

修复建议：

function resolveCodexAcpPlatformPackageSpecifier(packageName: string): string {
  return `${packageName}@${CODEX_ACP_BRIDGE_VERSION}`;
}

同步更新相关测试中的期望值。

🔵 LOW — lint: no-await-in-loop 在缓存扫描循环中

文件：src/process/agent/acp/acpConnectors.ts，第 409 行

问题代码：

const stat = await fs.stat(candidatePath);  // ← lint warning

问题说明：oxlint 报告 no-await-in-loop 警告。此处是顺序扫描多个缓存目录以找出最新二进制，可通过 Promise.allSettled 并行化以消除警告并提升性能。

🔵 LOW — lint: no-await-in-loop 在重试循环中（语义上合理但触发 lint）

文件：src/process/agent/acp/acpConnectors.ts，第 587 行、第 610 行

问题说明：oxlint 报 no-await-in-loop，但此处的顺序重试是语义要求。可通过 oxlint 行内注释抑制：

// oxlint-disable-next-line no-await-in-loop
await connectNpxBackend({ ... });

🔵 LOW — 直接变异候选包数组（违反不可变原则）

文件：src/process/agent/acp/acpConnectors.ts，第 614 行

问题代码：

codexPackageCandidates.push(fallbackPackage);  // ← mutation

问题说明：在 for...of 迭代中直接 .push() 变异数组，违反项目不可变原则。建议通过重新赋值新数组来扩展候选列表。

🔵 LOW — POSIX 缓存目录默认值不标准

文件：src/process/utils/shellEnv.ts，第 581 行

问题代码：

const npmCacheBase = posixCacheCandidates.find((candidate) => existsSync(candidate)) || posixCacheCandidates[0];

问题说明：两个候选都不存在时默认 ~/.npm-cache，但 npm POSIX 标准默认为 ~/.npm。建议改为 posixCacheCandidates[1] 作为回退。

汇总

结论

⚠️ 有条件批准 — 方案整体正确，测试覆盖充分；MEDIUM 问题（平台包版本未固定）有潜在协议不匹配风险，建议修复后合并。

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

PR Fix 验证报告

原始 PR: #1594
修复方式: 直接推送到 codex/fix-codex-acp-platform-fallback

总结： ✅ 已修复 1 个 | ❌ 未能修复 0 个

🔵 LOW 级别问题（4 个）已跳过（不阻塞合并，修复优先级低）。

To use Codex here, create a Codex account and connect to github.