fix: 兼容低版本 Node 启动预检#31
Conversation
There was a problem hiding this comment.
Code Review
此 PR 重构了 CLI 入口(src/cli.ts),通过动态导入延迟加载核心依赖,确保在低版本 Node.js 环境下优先执行版本守卫(src/lib/preflight.ts)并输出可读提示。同时,移除了 preflight.ts 中低版本 Node 不支持的 node: 协议导入和双问号(??)语法,并新增了兼容性测试。审查意见指出,测试文件中使用的正则表达式硬编码了单引号,若后续代码格式化将单引号转换为双引号,可能导致测试失效或误报,建议将正则匹配修改为同时兼容单双引号。此 PR 整体设计合理,能有效解决低版本 Node 运行时的崩溃问题,在修复测试中的正则硬编码问题后即可安全合并。置信度评分:5/5。
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| assert.doesNotMatch(source, /\?\?/, 'preflight must not use nullish coalescing before the Node version guard runs'); | ||
| assert.doesNotMatch(source, /\?\./, 'preflight must not use optional chaining before the Node version guard runs'); | ||
| assert.doesNotMatch(source, /from 'node:/, 'preflight must not use node: imports before the Node version guard runs'); |
There was a problem hiding this comment.
【P2 兼容性与测试漏洞风险】
当前测试中使用硬编码的单引号 from 'node: 来匹配 node: 协议导入。如果 TypeScript 编译器、格式化工具(如 Prettier)或未来的构建流程将生成的 JS 代码中的单引号转换为双引号(例如 from "node:fs"),此断言将无法匹配,从而导致测试静默通过(即无法检测出引入了 node: 导入的回归问题)。
建议将正则表达式修改为同时支持单引号和双引号,以提高测试的健壮性。
| assert.doesNotMatch(source, /\?\?/, 'preflight must not use nullish coalescing before the Node version guard runs'); | |
| assert.doesNotMatch(source, /\?\./, 'preflight must not use optional chaining before the Node version guard runs'); | |
| assert.doesNotMatch(source, /from 'node:/, 'preflight must not use node: imports before the Node version guard runs'); | |
| assert.doesNotMatch(source, /\?\?/, 'preflight must not use nullish coalescing before the Node version guard runs'); | |
| assert.doesNotMatch(source, /\?\./, 'preflight must not use optional chaining before the Node version guard runs'); | |
| assert.doesNotMatch(source, /from ['"]node:/, 'preflight must not use node: imports before the Node version guard runs'); |
| assert.match(source, /import '\.\/lib\/preflight\.js';/); | ||
| assert.doesNotMatch(source, /import \{ createProgram \} from '\.\/lib\/command\.js';/); | ||
| assert.doesNotMatch(source, /import \{ logger \} from '\.\/utils\/logger\.js';/); | ||
| assert.match(source, /import\('\.\/lib\/command\.js'\)/); | ||
| assert.match(source, /import\('\.\/utils\/logger\.js'\)/); |
There was a problem hiding this comment.
【P2 兼容性与测试漏洞风险】
类似于上述问题,这里的断言也硬编码了单引号。如果构建输出或格式化改变了引号类型(例如使用双引号),这些测试将会失败(对于 assert.match)或失效(对于 assert.doesNotMatch)。
建议使用支持单双引号的正则表达式,使测试更加健壮。
| assert.match(source, /import '\.\/lib\/preflight\.js';/); | |
| assert.doesNotMatch(source, /import \{ createProgram \} from '\.\/lib\/command\.js';/); | |
| assert.doesNotMatch(source, /import \{ logger \} from '\.\/utils\/logger\.js';/); | |
| assert.match(source, /import\('\.\/lib\/command\.js'\)/); | |
| assert.match(source, /import\('\.\/utils\/logger\.js'\)/); | |
| assert.match(source, /import ['"]\.\/lib\/preflight\.js['"];/); | |
| assert.doesNotMatch(source, /import \{ createProgram \} from ['"]\.\/lib\/command\.js['"];/); | |
| assert.doesNotMatch(source, /import \{ logger \} from ['"]\.\/utils\/logger\.js['"];/); | |
| assert.match(source, /import\(['"]\.\/lib\/command\.js['"]\)/); | |
| assert.match(source, /import\(['"]\.\/utils\/logger\.js['"]\)/); |
Codex 评审结论:通过 发现的问题 未发现需要阻塞的问题。本 PR 只改动启动入口与 Node 版本预检: 交互风险已结合报告核对:主菜单、工具选择器,以及 Claude Code、Codex、CodeBuddy、WorkBuddy、Hermes Agent 工具菜单均通过真实伪终端键盘导航检查;方向键选择、回车确认、返回/退出路径正常。报告中的重复渲染计数均在阈值内,未看到菜单持续闪烁、卡住或终端输出不可读的问题。交互测试脚本使用临时 验证说明 已读取并参考:
|
Codex 评审结论:通过 发现的问题 未发现需要阻断合并的问题。 本 PR 影响范围集中在启动入口和 Node 版本预检: 交互报告显示主菜单、工具选择器,以及 Claude Code、Codex、CodeBuddy、WorkBuddy、Hermes Agent 各工具菜单均通过键盘导航场景;未看到菜单持续闪烁、方向键失效、回车无法确认、卡住无法退出或终端输出不可读的问题。脚本也使用临时 HOME/USERPROFILE 夹具,未见污染真实 HOME 的迹象。 测试/验证
|
Summary
Test plan