fix(cli): derive bootstrap --help from shared option definitions - #8902
fix(cli): derive bootstrap --help from shared option definitions#8902yiliang114 wants to merge 4 commits into
Conversation
The bare `qwen --help` fast-path rendered a hand-maintained list that drifted from the real parser, hiding --approval-mode, --auth-type, and ~30 other accepted flags (issue #8897). Extract all non-hidden option definitions into a single shared module (top-level-options.ts) that both the bootstrap help parser and the real config parser consume from, keeping options in sync going forward. Also expand VALUE_FLAGS in cli.ts with every value-consuming option registered in the shared definitions, so resolveBootstrapRoute skips past values for these flags when detecting --help / --version.
|
Thanks for the PR! Template looks good ✓ Problem: observed bug, well evidenced — #8897 carries a concrete reproduction on 0.21.9 ( Direction: aligned. Size: touches core paths ( Approach: scope feels right. The hand-maintained bootstrap option list is the bug — adding the two missing flags to it would leave the drift mechanism in place, so deriving both parsers from shared definitions attacks the root cause. The part that deserves careful review is the Risk: no elevated risk signals (no high-risk paths matched). One practical note: the branch currently reports merge conflicts with Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，证据充分 —— #8897 提供了 0.21.9 上的具体复现（ 方向：对齐。 规模：触及核心路径（ 方案：范围合理。手工维护的 bootstrap 选项列表就是 bug 本身——只往里补两个参数会保留漂移机制，让两个解析器共享同一份定义才是根因修复。需要仔细 review 的部分是 bootstrap 路由里 风险：无升级风险信号（未命中高风险路径）。一个实际提醒：当前分支与 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: the hand-maintained bootstrap option list is the drift mechanism, so the fix has to make both parsers consume one set of definitions — extract the option metadata into a shared module, keep the runtime parsing bits ( What I verified in the code:
Non-blocking nits: the completeness claim ("all ~50 non-hidden options") has no test pinning it — a drift-guard asserting every registered non-hidden option shows up in help would keep this bug class dead. And the bootstrap help parser doesn't call No critical blockers found. Testing evidenceThis is an unattended CI run — no PR code was built or executed here; the evidence below is the PR's own CI read via the API. As of this review, no
Not verified: the full help surface at runtime and the Sandboxed verification would settle this: 中文说明代码审查读 diff 前我的独立思路：手工维护的 bootstrap 选项列表就是漂移机制本身，修复必须让两个解析器共用同一份定义——把选项元数据抽到共享模块，运行时解析部分（ 已核实的点：
非阻塞建议："全部约 50 个非隐藏选项"这一完整性主张没有测试钉住——可以加一个防漂移断言（每个已注册的非隐藏选项都出现在 help 中），让这类 bug 彻底绝迹。另外 bootstrap help 解析器没有调用 未发现关键阻塞问题。 测试证据这是无人值守的 CI 运行——此处未构建或执行任何 PR 代码；以下证据来自 PR 自身 CI 的 API 读取。 截至本次审查，该提交尚无任何 未验证：运行时的完整 help 输出面与 沙箱验证可以定论： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review on a genuinely needed fix; the cap is policy, not doubt — Stage 0 core-path escalation needs a maintainer's sign-off, and there is no CI signal or mergeability on the reviewed commit yet. Stepping back: this is the fix I'd have proposed. The hand-maintained bootstrap list was the bug, and making both parsers consume one set of option definitions kills the drift class rather than patching one more symptom — a separate issue two months from now would otherwise look exactly like #8897. The implementation is honest about what it is: mostly definitions moving to a shared module, with parsing behavior ( What keeps this at 3/5 rather than an approval:
So: no approval in this run, and no deferred-approval marker — the verdict is a human sign-off, not "wait for green". Details in the defer comment below. 中文说明置信度：3/5 —— 对一个确实需要的修复，审查是干净的；这个上限来自策略而非对代码的疑虑：Stage 0 核心路径升级需要维护者签核，且被审提交上还没有任何 CI 信号或可合并性。 退一步看：这正是我会提出的方案。手工维护的 bootstrap 列表本身就是 bug，让两个解析器共用同一份选项定义消灭了漂移这一类问题，而不是再补一个症状——否则两个月后会出现一个和 #8897 一模一样的 issue。实现也很坦诚：主要是定义移动到共享模块，解析行为（ 停在 3/5 而非批准的原因：
因此：本次运行不批准，也不发出延迟批准标记——结论是等待人工签核，而非"等 CI 变绿"。详见下方的 defer 评论。 — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Deferring to @yiliang114 (maintainer — author of this PR and no other human reviewer on the thread) — Stage 0 policy requires maintainer awareness before the bot can approve: 822 production lines touch the core path 中文说明⏸️ 移交 @yiliang114（维护者——本 PR 的作者，且线程上没有其他人类审查者）—— Stage 0 策略要求维护者知晓后 bot 才能批准：822 个生产行触及核心路径 — Qwen Code · qwen3.8-max |
|
Local UI verification: PASS
中文本地 UI 验证：PASS
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 2: none — did not run repo-wide typecheck, but the redundancy finding is verified by direct comparison of the two definitions and does not depend on it..
中文说明
未审查：build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 2：none — did not run repo-wide typecheck, but the redundancy finding is verified by direct comparison of the two definitions and does not depend on it.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| '--worktree', | ||
| '--max-session-turns', | ||
| '--max-wall-time', |
There was a problem hiding this comment.
[Critical] R1-1: The VALUE_FLAGS additions make hasFlag/firstPositionalArg swallow the -- end-of-options sentinel when it directly follows a bare value flag — both loops do an unconditional i++ for VALUE_FLAGS members before the arg === '--' guard is evaluated, so positionals after -- named --help/-h/--version/-v are treated as flags instead of prompt text. — Failure scenario: qwen --worktree -- --help (bare flag, sentinel, literal prompt --help) → the scan skips the -- token, detects --help, and prints help instead of starting a session. Probe-verified: PR code routes this to 'help'; with --worktree removed from the set it routes to 'default', matching pre-PR behavior where the input reached the full parser. The trigger is narrow, but it is a concrete wrong-action regression this diff introduces for all ~30 added flags.
R1-1 and R1-2 share one fix — in both scan loops, only consume the next token when it is neither the sentinel nor flag-like:
if (VALUE_FLAGS.has(arg)) {
const next = argv[i + 1];
if (next !== undefined && next !== '--' && !next.startsWith('-')) {
i++;
}
continue;
}中文说明
VALUE_FLAGS 的新增条目会让 hasFlag/firstPositionalArg 在裸值标志后紧跟 --（选项结束哨兵）时把它吞掉：两个循环对 VALUE_FLAGS 成员无条件执行 i++，在判断 arg === '--' 之前就已跳过该 token，导致 -- 之后名为 --help/-h/--version/-v 的位置参数被当作标志处理，而不是提示词。— 失败场景：qwen --worktree -- --help（裸标志 + 哨兵 + 字面提示词 --help）→ 扫描跳过 --，检测到 --help，打印帮助而不是启动会话。已通过探针验证：PR 代码将该输入路由为 'help'；从集合中移除 --worktree 后路由为 'default'，与 PR 前行为一致（输入会进入完整解析器）。触发条件较窄，但这是本 diff 为全部约 30 个新增标志引入的具体错误行为回归。
R1-1 与 R1-2 可用同一个修复解决：在两个扫描循环中，仅当下一个 token 既不是哨兵也不以 - 开头时才消费它（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| '--session-id', | ||
| '--worktree', |
There was a problem hiding this comment.
[Critical] R1-2: Adding --worktree to VALUE_FLAGS breaks the bootstrap --help/--version fast path for the documented bare --worktree form. The scan unconditionally consumes the token after --worktree as its value, but yargs itself never consumes flag-like tokens as string-option values (probed against this repo's yargs 17.7.2: ['--worktree', '--help'] parses as bare worktree: "" plus a live --help). — Failure scenario: qwen --worktree --help → --help is skipped as worktree's value, resolveBootstrapRoute returns 'default' instead of 'help' (pre-PR it returned 'help', verified at the merge base), so main() is imported and the full CLI graph boots before parseArguments's .help() prints help — defeating the fast path this PR exists to build. Output remains correct via the slow path; the defect is the introduced misrouting. Same shape for --worktree -h, --worktree --version, --worktree -v.
Fix together with R1-1: when skipping a VALUE_FLAGS entry in hasFlag/firstPositionalArg, only consume the next token if it is not -- and does not start with - (mirroring yargs-parser).
中文说明
将 --worktree 加入 VALUE_FLAGS 会破坏文档支持的裸 --worktree 形式的 bootstrap --help/--version 快速路径。扫描无条件地把 --worktree 后的 token 当作它的值消费，但 yargs 本身从不把形似标志的 token 作为字符串选项的值（已用本仓库的 yargs 17.7.2 探针验证：['--worktree', '--help'] 解析为裸 worktree: "" 加生效的 --help）。— 失败场景：qwen --worktree --help → --help 被当作 worktree 的值跳过，resolveBootstrapRoute 返回 'default' 而非 'help'（PR 前返回 'help'，已在合并基点验证），于是 main() 被导入、完整 CLI 启动图先运行，之后 parseArguments 的 .help() 才打印帮助——恰恰绕过了本 PR 要建立的快速路径。经由慢速路径输出仍然正确；缺陷在于新引入的错误路由。--worktree -h、--worktree --version、--worktree -v 同理。
与 R1-1 一并修复：在 hasFlag/firstPositionalArg 中跳过 VALUE_FLAGS 条目时，仅当下一个 token 不是 -- 且不以 - 开头时才消费它（与 yargs-parser 行为对齐）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| '--proxy', | ||
| '--system-prompt', | ||
| '--sandbox-image', |
There was a problem hiding this comment.
[Suggestion] R1-3: The VALUE_FLAGS expansion omits five value-taking options that the shared definitions register — --append-system-prompt, --telemetry-target, --telemetry-otlp-endpoint, --telemetry-otlp-protocol, --telemetry-outfile — while their sibling --system-prompt was added. — Failure scenario: qwen --append-system-prompt "be brief" --help → the value token is not skipped, is read as a positional, and the route becomes 'default': the full CLI graph boots before the real parser prints help, while the identical shape with --system-prompt gets the instant bootstrap help. Probe-verified: all five shapes route 'default'; adding the five entries flips them to 'help'. Output is correct when startup succeeds — a fast-path/consistency gap in the very set this PR exists to align. (Four review lenses reported this independently.)
| '--proxy', | |
| '--system-prompt', | |
| '--sandbox-image', | |
| '--proxy', | |
| '--system-prompt', | |
| '--sandbox-image', | |
| '--append-system-prompt', | |
| '--telemetry-target', | |
| '--telemetry-otlp-endpoint', | |
| '--telemetry-otlp-protocol', | |
| '--telemetry-outfile', |
中文说明
VALUE_FLAGS 的扩充遗漏了共享定义中注册的五个取值选项——--append-system-prompt、--telemetry-target、--telemetry-otlp-endpoint、--telemetry-otlp-protocol、--telemetry-outfile——而它们的同类 --system-prompt 却被加入了。— 失败场景：qwen --append-system-prompt "be brief" --help → 值 token 未被跳过，被当作位置参数，路由变为 'default'：完整 CLI 启动图先运行，之后真正的解析器才打印帮助；而用 --system-prompt 的相同形式却能立即走 bootstrap 快速路径。已探针验证：五种形式均路由为 'default'；补入这五个条目后全部变为 'help'。启动成功时输出仍然正确——这是本 PR 旨在对齐的集合中遗留的快速路径/一致性缺口。（四个审查视角独立报告了此问题。）
— qwen3.8-max via Qwen Code /review (v0.21.9)
| ...DEFAULT_COMMAND_OPTIONS.channel, | ||
| choices: ['VSCode', 'ACP', 'SDK', 'CI', 'desktop', 'daemon'] as const, |
There was a problem hiding this comment.
[Suggestion] R1-4: The inline choices override here is byte-for-byte identical to the list DEFAULT_COMMAND_OPTIONS.channel already carries, so it re-declares the one piece of data the new shared module exists to single-source. The override is not type-required — replacing the whole block with .option('channel', DEFAULT_COMMAND_OPTIONS.channel) passes tsc --noEmit with zero diagnostics (verified). — Failure scenario: if a new channel is later added only to the shared definition, qwen --help advertises it while the real parser's inline override rejects it with "Invalid values" — silently re-creating the exact issue-8897 divergence for this flag. This branch's own tip commit is a merge-conflict resolution over exactly this channel list. (Three review lenses reported this independently.)
.option('channel', DEFAULT_COMMAND_OPTIONS.channel)中文说明
此处的内联 choices 覆盖与 DEFAULT_COMMAND_OPTIONS.channel 已携带的列表逐字节相同，等于把新共享模块本要统一管理的唯一数据又声明了一遍。该覆盖并非类型所需——将整个块替换为 .option('channel', DEFAULT_COMMAND_OPTIONS.channel) 后 tsc --noEmit 零诊断通过（已验证）。— 失败场景：若将来新渠道只加入共享定义，qwen --help 会宣传它，而真实解析器的内联覆盖会以 "Invalid values" 拒绝它——在不知不觉间为这个标志重现 issue 8897 式的分歧。本分支的最新提交恰恰是一次关于这份 channel 列表的合并冲突解决。（三个审查视角独立报告了此问题。）
— qwen3.8-max via Qwen Code /review (v0.21.9)
| '--approval-mode', | ||
| '--auth-type', |
There was a problem hiding this comment.
[Suggestion] R1-6: The ~29 newly added VALUE_FLAGS entries have no test — the only route-resolution value-skip tests in cli.test.ts use the pre-existing --model / -p. — Failure scenario: a deletion probe was executed: removing '--approval-mode' from VALUE_FLAGS leaves cli.test.ts 55/55 green while routing flips (resolveBootstrapRoute(['--approval-mode','auto','--help']) goes 'help' → 'default'). The routing behavior this PR ships can silently regress with every test green.
expect(resolveBootstrapRoute(['--approval-mode', 'auto', '--help'])).toBe('help');
expect(resolveBootstrapRoute(['--auth-type', 'qwen-oauth', '--help'])).toBe('help');plus one array-flag case and a bare --worktree + --help case pinning whichever behavior is chosen for R1-2.
中文说明
新增的约 29 个 VALUE_FLAGS 条目没有任何测试——cli.test.ts 中仅有的路由解析值跳过测试用的是既有的 --model / -p。— 失败场景：已执行删除探针：从 VALUE_FLAGS 中移除 '--approval-mode' 后 cli.test.ts 仍为 55/55 全绿，但路由发生翻转（resolveBootstrapRoute(['--approval-mode','auto','--help']) 由 'help' 变为 'default'）。本 PR 交付的路由行为可以在所有测试全绿的情况下悄无声息地回归。
另建议补充一个数组标志用例，以及一个裸 --worktree + --help 用例，钉住 R1-2 最终选定的行为。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Verify flags that were historically missing from the bootstrap help | ||
| // (issue #8897). | ||
| expect(helpText).toContain('--approval-mode'); | ||
| expect(helpText).toContain('--auth-type'); |
There was a problem hiding this comment.
[Suggestion] R1-7: The invariant this PR exists to establish — "qwen --help lists every accepted non-hidden flag" — is guarded by 2 sampled substrings out of the ~44 flags newly surfaced; no test asserts parity between the registered options and the help listing. — Failure scenario: a future flag registered inline in config.ts (the pattern experimental-acp and sandbox-session-id use in this same diff), or any mutation narrowing TOP_LEVEL_HELP_OPTIONS, silently omits that flag from --help — re-creating issue 8897 for that flag — while all existing toContain assertions stay green.
for (const [name] of TOP_LEVEL_HELP_OPTIONS) {
expect(helpText).toContain(`--${name}`);
}中文说明
本 PR 要确立的不变量——"qwen --help 列出所有已接受的非隐藏标志"——目前只由约 44 个新出现标志中抽样的 2 个子串守护；没有测试断言已注册选项与帮助列表的一致性。— 失败场景：将来在 config.ts 中内联注册的标志（本 diff 中 experimental-acp 和 sandbox-session-id 使用的模式），或任何收窄 TOP_LEVEL_HELP_OPTIONS 的改动，都会让该标志悄无声息地从 --help 中消失——为该标志重现 issue 8897——而所有既有的 toContain 断言仍然全绿。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| '--extensions', | ||
| '-e', | ||
| '--include-directories', |
There was a problem hiding this comment.
[Suggestion] R1-8: The bootstrap scanner consumes exactly one token per VALUE_FLAGS entry, but seven flags this diff adds to the set are type: 'array' in the real parser, where yargs greedily consumes ALL following non-dash tokens: --extensions/-e, --include-directories/--add-dir, --allowed-mcp-server-names, --allowed-tools, --core-tools, --exclude-tools, --disabled-slash-commands. — Failure scenario: qwen --extensions ext1 ext2 --help → firstPositionalArg skips only ext1, sees ext2 as a positional, and the route becomes 'default': the full CLI graph boots before yargs prints help. Probe-verified end-to-end on the built CLI: fast-path --help takes 0.06s vs 0.64s for the misrouted array shape.
One way to fix — make the scan array-aware:
const ARRAY_VALUE_FLAGS = new Set([
'--extensions', '-e', '--include-directories', '--add-dir',
'--allowed-mcp-server-names', '--allowed-tools', '--core-tools',
'--exclude-tools', '--disabled-slash-commands', '--fallback-model',
]);
// in both scan loops:
if (ARRAY_VALUE_FLAGS.has(arg)) {
while (argv[i + 1] !== undefined && !argv[i + 1].startsWith('-')) i++;
continue;
}中文说明
bootstrap 扫描器对每个 VALUE_FLAGS 条目只消费恰好一个 token，但本 diff 加入该集合的七个标志在真实解析器中是 type: 'array'，yargs 会贪婪地消费其后所有非 - 开头的 token：--extensions/-e、--include-directories/--add-dir、--allowed-mcp-server-names、--allowed-tools、--core-tools、--exclude-tools、--disabled-slash-commands。— 失败场景：qwen --extensions ext1 ext2 --help → firstPositionalArg 只跳过 ext1，把 ext2 当作位置参数，路由变为 'default'：完整 CLI 启动图先运行，之后 yargs 才打印帮助。已在构建产物上端到端探针验证：快速路径 --help 耗时 0.06s，而被误路由的数组形式耗时 0.64s。
一种修复方式——让扫描感知数组标志（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
Fixed the bootstrap help option parsing follow-ups in |
|
Fixed the exact-head CI failure on |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): Context: PR 8902 derives the bootstrap qwen --help outp...: none — all checks above were completed within budget..
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：Context: PR 8902 derives the bootstrap qwen --help outp...：none — all checks above were completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| 'sandbox-image': | ||
| 'Use the "tools.sandboxImage" setting in settings.json instead. This flag will be removed in a future version.', | ||
| prompt: | ||
| 'Use the positional prompt instead. This flag will be removed in a future version.', |
There was a problem hiding this comment.
[Suggestion] R2-7: The newly derived primary qwen --help output is self-contradictory — the usage banner tells users to "use -p/--prompt for non-interactive mode", while this PR's new deprecateOption loop in buildTopLevelHelpParser makes the same output mark -p, --prompt as [deprecated: Use the positional prompt instead...]. The banner recommends the very flag the same screen marks for removal. — Failure scenario: an integrator following the banner adopts -p/--prompt for scripting → the same output's deprecation marker tells them it is slated for removal; contradictory guidance in the exact output this PR exists to make trustworthy. A/B-verified on the built CLI: merge-base qwen --help rendered 0 deprecation markers (banner consistent); this commit renders 9 (banner contradictory). The contradiction pre-existed on qwen <query> --help but is newly introduced on the primary path by this diff. The added toContain('deprecated') test shows the markers are intentional — fix the banner, not the marker: reword it in both parsers (a shared constant is preferred since cli.ts:140 and config.ts:567 duplicate the literal), e.g. "Qwen Code - Launch an interactive CLI. Pass the prompt as a positional argument, or use -i/--prompt-interactive for interactive mode."
中文说明
新推导出的主 qwen --help 输出自相矛盾：用法横幅告诉用户 "use -p/--prompt for non-interactive mode"，而本 PR 在 buildTopLevelHelpParser 中新增的 deprecateOption 循环又使同一输出把 -p, --prompt 标记为 [deprecated: Use the positional prompt instead...]。横幅推荐的正是同一屏中标记为即将移除的标志。— 失败场景：集成者按横幅在脚本中使用 -p/--prompt → 同一输出的弃用标记却提示该参数将被移除；在本 PR 要确立为可信的唯一输出中出现了互相矛盾的指引。已在构建产物上 A/B 验证：合并基点的 qwen --help 渲染 0 个弃用标记（横幅一致）；本提交渲染 9 个（横幅矛盾）。该矛盾在 qwen <query> --help 上早已存在，但本 diff 将其新引入主路径。新增的 toContain('deprecated') 测试表明标记是有意的——应修改横幅而非移除标记：在两个解析器中改写横幅（cli.ts:140 与 config.ts:567 重复同一字面量，建议提取共享常量），例如 "Qwen Code - Launch an interactive CLI. Pass the prompt as a positional argument, or use -i/--prompt-interactive for interactive mode."
— qwen3.8-max via Qwen Code /review (v0.21.9)
| for (const [name] of TOP_LEVEL_HELP_OPTIONS) { | ||
| expect(helpText).toContain(`--${name}`); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-1: This parity test iterates TOP_LEVEL_HELP_OPTIONS — the same array buildTopLevelHelpParser consumes — so drift between parseArguments's actual .option() registrations in config.ts and top-level-options.ts is invisible in both directions. — Failure scenario: (a) a flag added inline in the parseArguments chain (the pattern the hidden options experimental-acp/experimental-skills/sandbox-session-id use in this same diff) works but is omitted from qwen --help — re-creating issue 8897 — all tests green; (b) an entry added to DEFAULT_COMMAND_OPTIONS but never .option()-registered is advertised by help while the strict parser rejects it with "Unknown argument" — also green. Both sides of the assertion move with the shared array. Suggested fix: cross-check against an independent oracle, e.g. in config.test.ts probe parseArguments() with each --<name> (plus a valid value for value-taking options) and assert no "Unknown argument", or assert the constructed yargs instance's registered non-hidden keys match TOP_LEVEL_HELP_OPTIONS.
中文说明
该一致性测试遍历的 TOP_LEVEL_HELP_OPTIONS 正是 buildTopLevelHelpParser 消费的同一数组，因此 config.ts 中 parseArguments 实际的 .option() 注册与 top-level-options.ts 之间的漂移在两个方向上都不可见。— 失败场景：(a) 在 parseArguments 链中内联新增标志（本 diff 中隐藏选项 experimental-acp/experimental-skills/sandbox-session-id 使用的模式）→ qwen --new-flag 可用但 qwen --help 不显示它——重现 issue 8897——且所有测试全绿；(b) 向 DEFAULT_COMMAND_OPTIONS 新增条目但未 .option() 注册 → 帮助会宣传它，而严格解析器以 "Unknown argument" 拒绝——同样全绿。断言两侧随同一数组一起变动。建议修复：用独立的参照交叉检查，例如在 config.test.ts 中用每个 --<name>（取值选项附带合法值）探测 parseArguments() 并断言无 "Unknown argument"，或断言构造出的 yargs 实例已注册的非隐藏键与 TOP_LEVEL_HELP_OPTIONS 一致。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect(helpText).toContain('-o, --output-format'); | ||
| expect(helpText).toContain('-r, --resume'); | ||
| for (const [name] of TOP_LEVEL_HELP_OPTIONS) { | ||
| expect(helpText).toContain(`--${name}`); |
There was a problem hiding this comment.
[Suggestion] R2-4: The parity assertions use substring matching (toContain('--' + name)); probe-verified vacuous cases: --telemetry is satisfied by the --telemetry-target line and --openai-logging by --openai-logging-dir. — Failure scenario: a regression that drops or mangles exactly the --telemetry (or --openai-logging) help line leaves toContain('--telemetry') satisfied by the sibling line — the drift this test was added to catch ships green. Corrections to the original claim, probe-verified: --model is NOT vacuously satisfiable (--fallback-model contains -model, not --model), and --prompt/--sandbox are already covered by the stricter assertions at lines 268/270. Fix flip-verified: the regex below passes on intact help for every option and fails where the current toContain is blind.
| expect(helpText).toContain(`--${name}`); | |
| expect(helpText).toMatch(new RegExp('--' + name + '(?:\\s|,|$)')); |
中文说明
一致性断言使用子串匹配（toContain('--' + name)）；已探针验证的空洞情形：--telemetry 会被 --telemetry-target 一行满足，--openai-logging 会被 --openai-logging-dir 满足。— 失败场景：一次回归恰好删除或改坏了 --telemetry（或 --openai-logging）的帮助行，toContain('--telemetry') 仍被兄弟行满足——本测试要捕捉的漂移会带着全绿测试上线。对原说法的两处更正（均已探针验证）：--model 不会被空洞满足（--fallback-model 只包含 -model 而非 --model）；--prompt/--sandbox 已由 268/270 行更严格的断言覆盖。修复已做翻转验证：上面的正则对完整帮助的每个选项通过，在当前 toContain 失明处失败。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| config.type === 'string' || | ||
| config.type === 'number' || | ||
| config.type === 'array' |
There was a problem hiding this comment.
[Suggestion] R2-2: The config.type === 'number' disjunct is newly added behavior (the old hardcoded VALUE_FLAGS had no number flags) pulling --json-fd, --max-session-turns, --max-tool-calls, --max-subagent-depth into bootstrap value-skipping — but no test exercises a number-typed option. A mutation probe was executed: deleting this branch flips resolveBootstrapRoute(['--json-fd','3','--help']) from 'help' to 'default' while all 56 existing tests stay green. — Failure scenario: the branch is removed or broken in a future refactor → qwen --json-fd 3 --help misroutes to 'default': the full CLI graph boots before help prints, with every test green. Suggested fix (in cli.test.ts's matches yargs value scanning test): expect(resolveBootstrapRoute(['--json-fd', '3', '--help'])).toBe('help'); plus optionally a --max-session-turns variant.
中文说明
config.type === 'number' 分支是新增行为（旧的硬编码 VALUE_FLAGS 不含 number 标志），它把 --json-fd、--max-session-turns、--max-tool-calls、--max-subagent-depth 纳入引导阶段的值跳过——但没有任何测试覆盖 number 类型选项。已执行变异探针：删除该分支会使 resolveBootstrapRoute(['--json-fd','3','--help']) 由 'help' 翻转为 'default'，而既有 56 个测试仍全绿。— 失败场景：未来重构移除或破坏该分支 → qwen --json-fd 3 --help 被误路由为 'default'：完整 CLI 启动图先运行，之后才打印帮助，且所有测试全绿。建议修复（在 cli.test.ts 的 matches yargs value scanning 测试中）：expect(resolveBootstrapRoute(['--json-fd', '3', '--help'])).toBe('help');，可再加一个 --max-session-turns 变体。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| 'prompt', | ||
| 'Use the positional prompt instead. This flag will be removed in a future version.', | ||
| ) | ||
| .option('auth-type', DEFAULT_COMMAND_OPTIONS['auth-type']) |
There was a problem hiding this comment.
[Suggestion] R2-3: The auth-type choices are now hardcoded literals in top-level-options.ts, severing the compile-time link to the core AuthType enum. Values match exactly today (5/5), so no current behavior change — but the protection genuinely lost is: renaming a member, changing its string value, or removing it used to be caught at compile time at the choices site; now the literals silently diverge and yargs validates against stale strings. — Failure scenario: a future AuthType rename/value-change in core → qwen --auth-type <changed-value> is rejected by yargs choices (or a removed value still accepted) while loadCliConfig casts to the enum — silent CLI/runtime divergence with green CI. Round-1 R1-5 accepted the literal duplication (importing AuthType here would pull the core module graph into the bootstrap fast path) — this suggestion is compatible with that design: a parity test needs no runtime core import (test files already import core). Suggested fix: assert the shared choices set equals Object.values(AuthType) in config.test.ts, and/or add a short sync-requirement comment at the auth-type entry in top-level-options.ts.
中文说明
auth-type 的 choices 现在是 top-level-options.ts 中的硬编码字面量，切断了与 core AuthType 枚举的编译期关联。当前值完全一致（5/5），因此没有现行为变化——但真正失去的保护是：重命名成员、修改其字符串值或删除成员过去会在 choices 处被编译期捕获，现在字面量会悄然漂移，yargs 将按过时的字符串校验。— 失败场景：将来 core 中 AuthType 重命名/改值 → qwen --auth-type <changed-value> 被 yargs choices 拒绝（或已删除的值仍被接受），而 loadCliConfig 按枚举转型——CI 全绿下的静默 CLI/运行时分歧。第 1 轮 R1-5 已接受字面量复制（在此导入 AuthType 会把 core 模块图拉进引导快速路径）——本建议与该设计兼容：一致性测试不需要运行时 core 导入（测试文件本就导入 core）。建议修复：在 config.test.ts 中断言共享 choices 集合等于 Object.values(AuthType)，并/或在 top-level-options.ts 的 auth-type 条目处加一条简短的同步要求注释。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| for (const [option, message] of Object.entries( | ||
| TOP_LEVEL_DEPRECATED_OPTIONS, | ||
| )) { | ||
| yargsInstance.deprecateOption(option, message); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-5: Deprecation-hint parity between the two help paths is unguarded — the only coverage is expect(helpText).toContain('deprecated') (cli.test.ts:278) on the fast-path help. Both paths read this map today so they agree, but yargs 17.7.2 also forwards an inline { deprecated: 'msg' } option field to deprecateOption (verified in yargs-factory.js and rendered end-to-end by probe). — Failure scenario: a future definition gains an inline deprecated: '...' inside parseArguments (or a .deprecateOption() call appears outside this loop) → the real parser's qwen <positional> --help renders the hint while bootstrap qwen --help silently drops it — re-creating issue 8897's drift class in deprecation metadata, all tests green (the word stays present via the nine map-based markers). Suggested fix: for each key of TOP_LEVEL_DEPRECATED_OPTIONS, assert [deprecated: on that option's line in the fast-path help, and/or diff fast-path getHelp() against the real parser's help text.
中文说明
两条帮助路径之间的弃用提示一致性没有任何守护——唯一覆盖是快速路径帮助上的 expect(helpText).toContain('deprecated')（cli.test.ts:278）。目前两条路径读取同一份 map 所以一致，但 yargs 17.7.2 还会把内联的 { deprecated: 'msg' } 选项字段转发给 deprecateOption（已在 yargs-factory.js 中验证并端到端探针渲染）。— 失败场景：将来某个定义在 parseArguments 中获得内联 deprecated: '...'（或出现循环外的 .deprecateOption() 调用）→ 真实解析器的 qwen <positional> --help 渲染该提示，而引导期 qwen --help 悄然丢弃——在弃用元数据上重现 issue 8897 的漂移类别，且所有测试全绿（该单词仍由九个基于 map 的标记满足）。建议修复：对 TOP_LEVEL_DEPRECATED_OPTIONS 的每个键，断言快速路径帮助中该选项行包含 [deprecated:，和/或将快速路径 getHelp() 输出与真实解析器的帮助文本做差异比对。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| 'Use the "tools.sandboxImage" setting in settings.json instead. This flag will be removed in a future version.', | ||
| prompt: | ||
| 'Use the positional prompt instead. This flag will be removed in a future version.', | ||
| } as const satisfies Record<string, string>; |
There was a problem hiding this comment.
[Suggestion] R2-9: TOP_LEVEL_DEPRECATED_OPTIONS keys are free-form strings with no compile-time or test-time link to the option names they deprecate. satisfies Record<string, string> accepts any keys; yargs records deprecateOption for a never-registered key as inert metadata (probe-verified: no marker, no error), and no test asserts the reverse direction (deprecated key ⇒ registered option). Pre-PR the nine .deprecateOption() calls sat adjacent to each registration; the extraction into this separate map creates the invisible-orphan mode. — Failure scenario: a follow-up renames telemetry-outfile → telemetry-out-file in TOP_LEVEL_GLOBAL_OPTIONS → it compiles, both loops call deprecateOption on the stale key, and the renamed flag ships without its migration notice, all tests green. The fix below was tsc-probed: compiles cleanly for the current 9 keys, rejects a stale key with TS2353. Optionally also add a one-line parity assertion that every TOP_LEVEL_DEPRECATED_OPTIONS key appears in TOP_LEVEL_HELP_OPTIONS.
| } as const satisfies Record<string, string>; | |
| } as const satisfies Partial<Record<keyof typeof TOP_LEVEL_GLOBAL_OPTIONS | keyof typeof DEFAULT_COMMAND_OPTIONS, string>>; |
中文说明
TOP_LEVEL_DEPRECATED_OPTIONS 的键是自由字符串，与其弃用的选项名之间没有编译期或测试期关联。satisfies Record<string, string> 接受任意键；yargs 对从未注册的键调用 deprecateOption 只记录惰性元数据（已探针验证：无标记、无报错），且没有测试断言反向（弃用键 ⇒ 已注册选项）。PR 前九个 .deprecateOption() 调用紧邻各自注册；提取到这份独立 map 后产生了不可见的孤儿键模式。— 失败场景：后续把 TOP_LEVEL_GLOBAL_OPTIONS 中的 telemetry-outfile 重命名为 telemetry-out-file → 编译通过，两个循环对过时键调用 deprecateOption，重命名后的标志在失去迁移提示的情况下上线，所有测试全绿。下方修复已做 tsc 探针：对当前 9 个键编译通过，对过时键以 TS2353 报错。可另加一行断言：TOP_LEVEL_DEPRECATED_OPTIONS 的每个键都出现在 TOP_LEVEL_HELP_OPTIONS 中。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
No blocking code findings at The remaining merge gates are operational: the branch currently conflicts with |
|
Closeout from resolve-pr-comments automation: Changed: PR body now includes the repository template's Chinese details section. No code changes. Pending: existing review threads remain; no product code changed in this pass. |
3 participants
What this PR does
Derives the
qwen --helpbootstrap output from the same option definitions the real argument parser uses, so--helplists every accepted CLI flag instead of a stale hand-maintained subset.Extracts all non-hidden CLI option definitions into a new shared module (
packages/cli/src/config/top-level-options.ts). Both the bootstrap help parser (cli.ts->buildTopLevelHelpParser) and the real config parser (config.ts) now consume from this single source of truth.Why it's needed
qwen --helplisted only 9 options while the real parser accepts around 50.--approval-modeand--auth-typewere both accepted and validated but invisible in help output, making headless integration confusing. The error messagePlease configure an auth type (e.g. via settings or \--auth-type`)pointed at a flag its own--help` never mentioned.Fixes #8897.
Reviewer Test Plan
How to verify
Before this PR: no output. After: both flags appear in the help text.
Unit test:
npx vitest run packages/cli/src/cli.test.ts -t "prints top-level help"asserts--approval-modeand--auth-typeare present.Evidence (Before & After)
Before:
--helpshows 9 options (model, fallback-model, prompt, prompt-interactive, safe-mode, sandbox, output-format, continue, resume).After:
--helpshows all around 50 non-hidden options registered in the real parser, including--approval-mode,--auth-type,--yolo,--debug,--mcp-config,--extensions,--channel,--worktree, telemetry flags, and more.Tested on
Environment (optional)
Not applicable.
Risk & Scope
VALUE_FLAGSexpansion incli.tscould alter bootstrap route detection for flags that consume a value argument. Mitigated by adding only flags whose type isstring/number/array(i.e. consume the next argv element), not booleans.config.test.tssuite. Theconfig.tsrefactor preserves the original structure: the same option objects are passed to yargs, just sourced from a different location. The 19 pre-existingreadFileSynctest failures on main are unchanged.Linked Issues
Fixes #8897.
中文说明
本 PR 让 qwen --help 从真实 CLI 选项定义生成，避免帮助输出遗漏已支持的参数。风险集中在 bootstrap 阶段的 value flag 判断，已通过聚焦测试验证。修复 #8897。