feat: 自动提醒更新 CLI 版本 - #11
Conversation
mizzenbot
left a comment
mizzenbot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating：可接受
**Linus-Style Analysis：**目标明确，且 stderr 与 stdout 的隔离处理正确；但为避免阻塞而 unref 请求 socket，会让通常短命令在响应到达前直接退出，因此核心的首次版本查询无法写入缓存。
[CRITICAL ISSUES]
src/update-notifier.ts：后台请求未能在 CLI 退出后继续完成，更新提醒在无既有缓存的用户端实际不会生效。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
核心用户可见功能在普通短生命周期命令中失效；无凭证、自动安装或 stdout 污染风险。
VERDICT：
❌ Needs rework：先让首次检查能可靠完成并落盘。
KEY INSIGHT：
“后台且不阻塞”不能通过 unref 正在执行的唯一网络请求实现，否则结果没有任何存活载体来写入缓存。
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.- Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Resolve with AI? Install the iterate skill in your agent and run
/iterateto automatically drive this PR through CI, review, and QA until it is merge-ready.Was this review helpful? React with 👍 or 👎 to give feedback.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/31405817218
| }) | ||
| }) | ||
|
|
||
| request.on("socket", (socket) => socket.unref()) |
There was a problem hiding this comment.
🟠 **重要：**这里解除唯一 socket 的引用后，短命令的事件循环会在 npm 响应到达前退出；response / end 回调因此不会运行，首次检查既不会写入 update-state.json，也不会在后续调用显示提示。我以 env -u CI HOME=$(mktemp -d) node dist/index.js --version 复现：命令立即返回，等待后仍不存在缓存文件。请使用在父进程退出后仍能存活的独立后台工作者，或重新设计检查时机；不要 unref 唯一负责写缓存的请求。
mizzenbot
left a comment
mizzenbot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating：可接受
**Linus-Style Analysis：**上一轮 unref 导致首次检查无法落盘的问题已修复，但修复方式把后台检查变成了所有命令的前置同步网络请求。
[CRITICAL ISSUES]
- 见
src/index.ts的行内意见：网络失败会让每次缓存缺失或过期的 CLI 调用阻塞最多 1.5 秒。
[TESTING GAPS]
- 需要增加缓存缺失且网络超时情况下，
--help或--version仍能立即完成的端到端回归测试。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
无凭证、自动安装或 stdout 污染风险；但网络不可用时所有日常 CLI 调用都会产生显著延迟。
VERDICT：
❌ Needs rework：恢复不阻塞原命令的检查语义，同时保证首次结果可可靠写入缓存。
KEY INSIGHT：
后台任务必须既不绑定主 CLI 进程的生命周期，也不能处于其关键调用路径。
改进这份审查？ 如果以上反馈有误或不适用于此仓库，可在分支中编辑
.agents/skills/custom-codereview-guide.md并添加/codereview触发器，随后重新请求审查；审查器会读取该分支的仓库规则。用 AI 解决？ 可安装 iterate skill，运行
/iterate自动推进 CI、审查和 QA。
此审查由 AI agent（OpenHands）代表用户生成。
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/31406339384
|
|
||
| const program = createProgram() | ||
| const version = loadVersion() | ||
| const update = await checkForUpdate(version) |
There was a problem hiding this comment.
🟠 **重要：**上一轮解除 socket 引用导致首次检查无法落盘的问题虽已解决，但这里在解析任何命令前等待网络请求，使无缓存或缓存过期的 --help、--version 和正常命令在 DNS/网络不可用时都额外阻塞最多 1.5 秒。这违反了 PR 承诺的“后台查询”和“网络失败不影响原命令”。请将同步路径限制为读取已有缓存，并把刷新交给主进程退出后仍能完成写入的独立工作者（或调整为其他不阻塞的检查时机）；同时覆盖超时网络下命令立即完成的回归测试。
mizzenbot
left a comment
mizzenbot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating：可接受
**Linus-Style Analysis：**独立进程使刷新脱离原 CLI 生命周期，已解决此前的同步阻塞与 unref 过早退出问题；但新增回归测试没有遵守自身的 CI 禁用语义，导致测试套件在 CI 环境失败。
[CRITICAL ISSUES]
- 见
src/update-notifier.test.ts行内意见：bun test当前失败，不能作为该功能的合并验证。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
不涉及凭证、自动安装或 stdout 污染，且异步刷新路径不阻塞主命令；但 CI 中的测试失败，无法可靠防止该用户可见行为回归。
VERDICT：
❌ Needs rework：修正测试的 CI 环境处理，并让回归测试覆盖实际的独立工作者/CLI 路径。
KEY INSIGHT：
测试必须在功能宣称会跳过更新检查的 CI 环境中保持自洽，否则它不能证明后台刷新实现正确。
改进这份审查？ 如果以上反馈有误或不适用于此仓库，可在分支中编辑
.agents/skills/custom-codereview-guide.md并添加/codereview触发器，随后重新请求审查；审查器会读取该分支的仓库规则。用 AI 解决？ 可安装 iterate skill，运行
/iterate自动推进 CI、审查和 QA。
此审查由 AI agent（OpenHands）代表用户生成。
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/31406805909
| const path = join(mkdtempSync(join(tmpdir(), "mizzen-update-")), "missing.json") | ||
| let scheduled = false | ||
|
|
||
| expect(checkForUpdate("0.2.0", path, () => { scheduled = true })).toBeNull() |
There was a problem hiding this comment.
🟠 重要：checkForUpdate 在 CI 非空时会于 src/update-notifier.ts:124 直接返回，因此此测试在 CI 中不会调用传入的回调，行 45 必然失败。我以锁文件中的 Bun 1.3.12 运行 bun test src/update-notifier.test.ts 复现为 2 passed / 1 failed。请像上一测试一样保存并清除 CI 后再断言调度；并增加经由真实 CLI/独立 worker 的回归用例，验证修复过的“父进程立即退出后仍可落盘”行为。
mizzenbot
left a comment
mizzenbot
left a comment
There was a problem hiding this comment.
🟢 Taste Rating：好品味
**Linus-Style Analysis：**刷新被隔离到独立工作者，主命令只读取本地缓存；短命令不会等待网络，提醒仍只写入 stderr。此前三项意见均已由当前差异解决，未发现新的重要行为风险。
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
不涉及凭证或自动安装；缓存目录和文件权限受限，网络失败仅影响后台缓存。已验证完整测试、类型检查、打包及打包 Node CLI 的慢网络退出/异步落盘路径。
VERDICT：
✅ Worth merging：核心逻辑可靠，可合并。
KEY INSIGHT：
独立工作者同时满足后台刷新与父进程快速退出的约束。
本审查由 AI 代理（OpenHands）代表用户生成。
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/MizzenAI/mizzen-cli/actions/runs/31407612611
2 participants
改动
npm update -g @mizzenai/cli验证
bun test：12 passedbun run typecheckbun run bundlegit diff --check