Skip to content

fix: 兼容低版本 Node 启动预检#31

Merged
krisxia0506 merged 2 commits into
mainfrom
codex/add-pr-review
Jun 6, 2026
Merged

fix: 兼容低版本 Node 启动预检#31
krisxia0506 merged 2 commits into
mainfrom
codex/add-pr-review

Conversation

@krisxia0506

Copy link
Copy Markdown
Collaborator

Summary

  • Keep the startup preflight parseable on older Node versions so unsupported runtimes see the Node >= 18 guidance instead of a syntax error.
  • Defer CLI application imports until after preflight runs to avoid static ESM parsing of newer syntax before the guard exits.
  • Add regression coverage for preflight syntax and deferred CLI imports.

Test plan

  • pnpm test
  • Docker: node:12-bullseye reproduces qiniu-coding-helper@0.3.5 syntax error and current tarball shows Node >= 18 guidance
  • Docker: node:14-alpine current tarball shows Node >= 18 guidance
  • Docker: node:16-bullseye current tarball shows Node >= 18 guidance
  • Docker: node:18-bullseye current tarball prints version successfully

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread tests/preflight-compat.test.mjs Outdated
Comment on lines +8 to +10
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');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

【P2 兼容性与测试漏洞风险】
当前测试中使用硬编码的单引号 from 'node: 来匹配 node: 协议导入。如果 TypeScript 编译器、格式化工具(如 Prettier)或未来的构建流程将生成的 JS 代码中的单引号转换为双引号(例如 from "node:fs"),此断言将无法匹配,从而导致测试静默通过(即无法检测出引入了 node: 导入的回归问题)。

建议将正则表达式修改为同时支持单引号和双引号,以提高测试的健壮性。

Suggested change
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');

Comment thread tests/preflight-compat.test.mjs Outdated
Comment on lines +16 to +20
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'\)/);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

【P2 兼容性与测试漏洞风险】
类似于上述问题,这里的断言也硬编码了单引号。如果构建输出或格式化改变了引号类型(例如使用双引号),这些测试将会失败(对于 assert.match)或失效(对于 assert.doesNotMatch)。

建议使用支持单双引号的正则表达式,使测试更加健壮。

Suggested change
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['"]\)/);

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Codex 评审

结论:通过

发现的问题

未发现需要阻塞的问题。本 PR 只改动启动入口与 Node 版本预检:src/cli.ts 将命令路由和 logger 改为 preflight 之后动态加载,src/lib/preflight.ts 避免低版本 Node 解析不兼容语法。未直接改动命令、向导 flow、菜单、工具适配器或配置写入路径。

交互风险已结合报告核对:主菜单、工具选择器,以及 Claude Code、Codex、CodeBuddy、WorkBuddy、Hermes Agent 工具菜单均通过真实伪终端键盘导航检查;方向键选择、回车确认、返回/退出路径正常。报告中的重复渲染计数均在阈值内,未看到菜单持续闪烁、卡住或终端输出不可读的问题。交互测试脚本使用临时 HOME/USERPROFILE,未发现污染真实 HOME 的迹象。

验证说明

已读取并参考:

  • codex-unit-test.logpnpm test 成功,包含新增 preflight 兼容性测试。
  • codex-interactive-test.logpnpm test:interactive 成功。
  • codex-interactive-report.md:覆盖主菜单、工具选择器和所有动态注册工具菜单的伪终端场景,均为 passed。

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

未发现新的需要指出的问题。

已在本地执行 pnpm test,36 个测试全部通过。

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Codex 评审

结论:通过

发现的问题

未发现需要阻断合并的问题。

本 PR 影响范围集中在启动入口和 Node 版本预检:src/cli.ts 将 CLI 应用逻辑改为预检后动态导入,src/lib/preflight.ts 移除了低版本 Node 可能无法解析的 node: import 和 ??。未改动菜单、向导 flow、工具适配器或配置写入路径。

交互报告显示主菜单、工具选择器,以及 Claude Code、Codex、CodeBuddy、WorkBuddy、Hermes Agent 各工具菜单均通过键盘导航场景;未看到菜单持续闪烁、方向键失效、回车无法确认、卡住无法退出或终端输出不可读的问题。脚本也使用临时 HOME/USERPROFILE 夹具,未见污染真实 HOME 的迹象。

测试/验证

  • pnpm test: success,36 个子测试通过,包含新增的 preflight 可解析性和 CLI 延迟导入回归测试。
  • pnpm test:interactive: success,交互报告中全部 7 个伪终端场景均为 passed。

@krisxia0506 krisxia0506 merged commit b0c0d0b into main Jun 6, 2026
6 checks passed
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.

1 participant