fix(cli): stop bootstrap help/version intercepts from swallowing a subcommand's free-text argument (#11193) - #11223
Conversation
…rcepts A free-text answer to `qwen sessions answer <session>` that quotes `--help`/`--version` (or ends in a bare `help`) was swallowed by the bootstrap scans and the root parser's help/version handling: the CLI printed the usage block or the version and exited 0, so a script driving a background session read success while the answer was never delivered. Insert a single `--` after the session id, in both the bootstrap route scan and parseArguments' raw argv, so every later token is positional data to those scans and to the yargs parse. The carve-outs stay: a bare `--help` payload (and `sessions answer --help` with no id) keeps showing the command's help, a user-supplied separator is never doubled, and the fail-closed version intercept for every other command is untouched. Fixes QwenLM#11193
|
Thanks for the PR — and for a description that is unusually precise about what it does and does not cover. Template looks good ✓ Problem: observed, not theoretical. #11193 carries a witness table (usage block + exit 0 for Direction: aligned. This is the "carve the payload out before yargs sees it" shape the issue itself proposes, and it correctly refuses the blanket exemption the issue rules out — demoting the version intercept executes subcommands, and Size: core path touched — Approach: the scope feels right, and it matches what I proposed independently before reading the diff — one shared helper applied at the two places that already rewrite raw argv, rather than teaching each scan about free text or restructuring the bootstrap to defer its intercepts. The prefix gate is strict enough that I could name every consumer: the fenced argv is local to Risk: no elevated risk signals — the Stage 1e path scan matched nothing. The real risk is the invisible contract with #10949: this injects a 中文说明感谢贡献！PR 描述对自己覆盖与未覆盖的范围写得异常清楚，这一点很难得。 模板完整 ✓ 问题：是已观测到的 bug，不是理论性加固。#11193 带有见证表格（ 方向：对齐。这正是 issue 自己提出的"在 yargs 看到之前把负载剥出来"方案，并且正确地拒绝了 issue 排除的一刀切豁免——降级 version 拦截会执行子命令，已观测到 规模：触及核心路径—— 方案：范围合理，也与我读 diff 之前独立提出的方案一致——用一个共享 helper 应用在两处已经重写 raw argv 的地方，而不是教会每个扫描理解自由文本，或重构 bootstrap 把拦截推迟。前缀匹配足够严格，我能点名每一个下游消费者：栅栏化后的 argv 只在 风险：无升级风险信号——Stage 1e 路径扫描未命中任何条目。真正的风险是与 #10949 之间的隐形契约：本 PR 注入的 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI wrote my independent proposal before opening the diff: fence the payload with a No critical blockers. I traced the mechanics rather than taking the description's word for them:
Three non-blocking notes: 1. The contract with #10949 is the thing nothing here can pin. This injects a 2. Two of the four assertions in the new 3. One thing I checked that is not a finding: a payload that is exactly sequenceDiagram
participant P1 as CLI argv
participant P2 as resolveBootstrapRoute
participant P3 as insertSessionAnswerSeparator
participant P4 as version and help scans
participant P5 as parseArguments
participant P6 as yargs root parse
P1->>P2: sessions answer id plus answer text
P2->>P3: normalize serve fast path, then fence
P3-->>P2: separator inserted after the session id
P2->>P4: scan the fenced argv
P4-->>P2: scans stop at the separator, route is default
P2->>P5: fall through to the full parser
P5->>P3: fence rawArgv a second time
P3-->>P5: same fenced shape, no doubling
P5->>P6: hand rawArgv to yargs
P6-->>P5: post separator tokens stay positional, no intercept fires
Testing evidenceThis is an unattended CI run, so per the skill's rules I did not build or execute anything from this PR — the evidence below is the PR's own CI, read through the API at the reviewed commit. Nothing is red. Three substantive checks are still running, and I'm reporting them as pending rather than guessing: Not verified, and why:
Sandboxed verification would settle the second bullet: Real-scenario tmux testing: N/A — unattended CI run, which never drives the product locally. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查我在打开 diff 之前先写了自己的独立方案：在两处已经重写 raw argv 的地方用 没有关键阻塞项。 我是实际追踪机制得出结论，而不是采信描述：
三点非阻塞意见： 1. 与 #10949 的契约是这里唯一钉不住的东西。 本 PR 注入的 2. 新增 3. 有一处我核查过但不算问题：恰为 时序图见上（参与者与标签保持英文，此处以文字概述）：argv 先进入 测试证据这是无人值守的 CI 运行，因此按 skill 规则我没有构建或执行本 PR 的任何代码——以下证据是 PR 自身的 CI，通过 API 在被审查的 commit 上读取。没有红色项。三个实质性检查仍在运行，我如实报告为 pending 而不做猜测： 未验证项及原因：
沙箱验证可以解决第二条： 真实场景 tmux 测试：N/A——无人值守 CI 运行从不在本地驱动产品。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the code holds up and I found no defects, but the promise it makes can't be checked on this branch and the call on sequencing belongs to a human. Stepping back. The approach is the one I'd have written independently, the helper is five guard clauses and a splice rather than something trying too hard, and there's no drive-by churn in the diff. I verified the problem is real rather than accepting the PR's framing: #11193 carries observed witnesses, and I read What stops me approving is the six-months-from-now question, and it cuts both ways. If #10949 lands as written, this is the right layering: the fence sits where raw argv is already rewritten, the long doc comment explains why, and the injected separator is stripped by the command's own tail recovery. If #10949 is reshaped or closed, this is a 67-line helper guarding a command that doesn't exist, injecting a Worth being clear about what this does change today, because it's smaller than the description's framing suggests. The volume context is worth naming, even though it isn't a mark against this PR specifically. This author has 19 open PRs, 16 of them created today, against 3 merged. I evaluated this one on its merits and it stands on its own. But it is also the shape the gate is supposed to be slowest about: well-researched, plausible, and scaffolding for a feature that hasn't landed. With maintainer attention as the scarce resource, that argues for a human deciding whether this merges ahead of, alongside, or after #10949 rather than the gate approving it because no defect turned up. Two smaller things a maintainer may want folded into whichever PR owns the command: the assertion that the delivered answer carries no leading separator, and the fact that two of the four new Deferring — not approving, and not requesting changes. I found no correctness, security, or regression defect, so request-changes would be the wrong signal; but Stage 0 Tier 2 escalation on the core path plus the unverifiable central claim cap this at 3/5, so I'm not approving either. CI is also still running on this commit ( I'm escalating this to a maintainer, and I want to be explicit that the deterministic resolver returned nobody: the PR carries no area labels and has no prior human reviewer, so there was no accountable owner to pick, and I'm not going to guess a login. Whoever owns the CLI argv/bootstrap surface, this one needs your call — specifically on merge order relative to #10949 and on which PR owns the verbatim-delivery test. Adding an area label such as 中文说明Confidence: 3/5 —— 代码站得住，我没有发现缺陷，但它所做的承诺在本分支上无法验证，而时序判断应该由人来做。 退一步看整体。方案与我独立写出的方案一致；helper 是五条守卫加一次拼接，没有用力过猛；diff 里也没有顺手夹带的改动。我核实了问题真实存在，而不是采信 PR 的叙述：#11193 带有观测到的见证，并且我在 让我不批准的是"六个月后回看"这个问题，而它是双向的。如果 #10949 按其书写落地，这就是正确的分层：栅栏位于已经在重写 raw argv 的地方，长长的文档注释解释了原因，注入的分隔符由命令自身的尾部恢复逻辑剥掉。如果 #10949 被改写或关闭，那么这就是一个 67 行、守护着不存在命令的 helper，注入一个没人移除的 需要说清楚它今天确实改变了什么，因为这比描述的框架要小。 体量的背景值得点名，尽管它本身不是针对这个 PR 的扣分项。该作者有 19 个 open PR，其中 16 个创建于今天，而已合并的是 3 个。我是按本身的价值评估这一个的，它站得住。但这也正是闸门应当最慢放行的形状：研究充分、看起来合理、并且是为尚未落地的功能搭的脚手架。在维护者注意力是稀缺资源的前提下，这更支持由人来决定它是先于、同时还是后于 #10949 合并，而不是因为没查出缺陷就由闸门批准。 有两件小事，维护者也许想并入最终拥有该命令的那个 PR：一是断言送达的回答不带前导分隔符；二是新增 转交维护者——不批准，也不 request changes。 我没有发现正确性、安全性或回归缺陷，所以 request-changes 是错误的信号；但核心路径上的 Stage 0 Tier 2 升级，加上无法验证的核心主张，把这一项封顶在 3/5，所以我也不批准。CI 在这个 commit 上也仍在运行（ 我正在把这个转交给维护者，并且要明确说明：确定性解析器没有返回任何人。该 PR 没有领域标签，也没有先前的人类审阅者，因此没有可选出的问责 owner，而我不会去猜一个登录名。无论谁负责 CLI argv/bootstrap 这一块，这一项需要你的判断——具体是关于相对 #10949 的合并顺序，以及由哪个 PR 拥有"原样送达"的测试。加上诸如 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
未审查：反向审计——评审时间预算不足，未能开始第 6 轮。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| if (argv.length === 4 && (argv[3] === '--help' || argv[3] === '-h')) { | ||
| return argv as string[]; | ||
| } | ||
| return [...argv.slice(0, 3), '--', ...argv.slice(3)]; |
There was a problem hiding this comment.
[Critical] R1-16: [certifies-falsely] [regression] Inserting the separator relocates the payload out of yargs' _, which makes the session-id slot the new trailing positional — so qwen sessions answer help <text> now prints help and exits 0 with an empty stderr, where the merge base failed loudly with exit 1. That re-opens, on a new trigger, the exact silent-success harm #11193 was filed about: a driving script (qwen sessions answer "$id" ... || fail) reads success while nothing was delivered.
The mechanism is in the shipped dependency. yargs forces 'populate--': true on every internal parse (yargs-factory.js:1339), so post--- tokens never reach _; :1371-1372 then pops argv._[argv._.length - 1] when it equals the root .help() spelling and sets helpOptSet, giving showHelp('log'); exit(0). With the payload fenced off, _ ends at the id slot, so an id slot spelling help gets popped. The pop runs at the root parse before dispatch, so this survives #10949 landing the real answer command. Swept across id-slot words, exactly one triggers it — lowercase help (h, version, HELP, ---help all exit 1; --help exits 0 identically on base). Session ids are UUID-shaped (config/session-id.ts), so no live session can be named help and no real answer is lost today; what is wrong is the exit-code and output-stream contract on an invalid command line.
Witness — base-tree A/B, identical argv on both arms (arm proof: grep -c insertSessionAnswerSeparator packages/cli/dist/src/config/config.js gives 2 on the PR, 0 on base):
BASE 92a8a8d17 argv=[sessions,answer,help,me] rc=1 stdout=0 stderr=2881 tail="Unknown arguments: answer, help, me"
PR f220d6763 argv=[sessions,answer,help,me] rc=0 stdout=2844 stderr=0 first="qwen sessions"
Keep the id slot out of the pop's reach so that shape stays on the loud parser path — the change is at line 52, not here:
if (session === undefined || session.startsWith('-') || session === 'help') {
return argv as string[];
}The tradeoff is explicit and acceptable: an id of help then loses fence protection for help/version tokens in its payload, which is strictly better than a silent exit 0 because help can never name a live session. If you would rather keep the fence unconditional, record the residual instead — a docstring line under the carve-outs plus a characterization test pinning exit 0 — so it reads as a known deferral rather than an undocumented consequence.
Two premises the fix must not violate: config.test.ts:353-377 pins the bare---help carve-out, asserting ['sessions','answer','0f8e1c42','--help'] still rejects with 'process.exit unexpectedly called with "0"' and prints output containing qwen sessions; and the pop cannot be closed from the answer command's builder, because per #11193 intercept 2 is keyed on the root instance's .help(), so no change inside a subcommand builder can close it. This guard line is also where the R1-13 fix lands, so please coordinate the two edits rather than applying them independently.
Please add the pinning test in config.test.ts beside the two new ones — process.argv = ['node','script.js','sessions','answer','help','me'], asserting rejects.toThrow('process.exit unexpectedly called with "1"') — and then prove it by removing the session === 'help' clause and confirming that test goes red. It is red at this commit (measured EXIT:0) and green with the guard.
中文说明
插入分隔符会把负载移出 yargs 的 _，于是 session id 槽位变成新的末尾 positional——qwen sessions answer help <text> 现在会打印帮助并以 exit 0 退出、stderr 为空，而在合并基线上它是响亮地 exit 1。这在一个新的触发形状上重新打开了 #11193 所要消除的“静默成功”危害：驱动脚本（qwen sessions answer "$id" ... || fail）读到成功，而回答根本没有送达。
机制在已发布的依赖里：yargs 每次内部解析都强制 'populate--': true（yargs-factory.js:1339），所以 -- 之后的 token 永远进不了 _；随后 :1371-1372 在 argv._[argv._.length - 1] 等于根 .help() 拼写时把它 pop 掉并置 helpOptSet，于是 showHelp('log'); exit(0)。负载被栅栏化后 _ 止于 id 槽位，因此拼作 help 的 id 槽会被 pop。该 pop 发生在根解析阶段、早于命令分发，所以即使 #10949 落地真正的 answer 命令它依然存在。对 id 槽词穷举后只有小写 help 一个会触发（h、version、HELP、---help 均 exit 1；--help 在基线上同样 exit 0）。session id 是 UUID 形状（config/session-id.ts），因此没有真实会话能叫 help，今天也不会真的丢失回答；错的是非法命令行上的退出码与输出流契约。
证据为与基线树的 A/B（两侧 argv 完全相同；分支证明见英文部分），输出块见上方英文部分。
建议让 id 槽避开这个 pop，使该形状留在响亮的解析器路径上——改动在第 52 行，而不是这一行。取舍是明确且可接受的：id 为 help 时其负载中的 help/version token 会失去栅栏保护，而这严格优于静默 exit 0，因为 help 永远不可能指向真实会话。如果更希望保持栅栏无条件生效，那就把残留记录下来——在 carve-out 列表里加一行 docstring，并补一个钉住 exit 0 的 characterization 测试——让它读起来是已知延后项而非未被记录的后果。
修复不得违反两个前提：config.test.ts:353-377 钉住了 bare---help carve-out；且该 pop 无法从 answer 命令的 builder 内关闭，因为按 #11193，拦截 2 以根实例的 .help() 为键。这一行守卫同时也是 R1-13 的修复落点，请协调两处编辑。
请在 config.test.ts 两个新用例旁补上钉住用的测试，然后通过移除 session === 'help' 子句、确认该测试变红来证明它有效。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| * Fence off the free-text payload of `qwen sessions answer <session>` from | ||
| * every flag scan that runs before and during the yargs parse. | ||
| * | ||
| * `sessions answer` is the one command whose positional tail is arbitrary |
There was a problem hiding this comment.
[Suggestion] R1-6: This docstring's uniqueness claim is the sentence that scopes the whole fence to one argv prefix, and two sibling commands in this tree contradict it. The default command's variadic prompt positional (DEFAULT_COMMAND = '$0 [query..]', config/top-level-options.ts:79) is the same arbitrary-prose tail shape and stays unfenced: on the built CLI, please --help me prints the top-level usage block and exits 0 with an empty stderr, the prompt never running — the same silent swallow this comment attributes to sessions answer alone. And mcp add <name> <commandOrUrl> [args...] (commands/mcp/add.ts:198) has the same tail, with cli.ts:452's hasFlag(argv.slice(2), '--help', '-h') firing against it.
The cost is twofold. A maintainer reading this comment concludes no other command has an arbitrary-prose tail, so both twin paths keep swallowing and nobody looks for them. And the mcp add row is a live silent-exit-0 instance today that an unqualified Fixes #11193 would close the issue over with nothing tracking it — #11065 is open at priority/P3 and status/blocked and owns scan consolidation, not this behaviour. Issue #11193's own triage record already names that row: "the class is already reachable on main, not gated on #10949 ... qwen mcp add <name> <cmd> [args...] --help -> mcp fast path prints help, exit 0, server never added ... Suggest amending the 'What' section so the fix is scoped against the live surface too". This is not already discussed on the PR — grep -i "mcp add" over all four existing PR comments returns zero hits.
Witness — both siblings run at the reviewed commit, the second against a build containing this PR's fence (grep -c insertSessionAnswerSeparator dist/src/cli.js gives 2), in a throwaway HOME:
node packages/cli/dist/index.js please --help me
-> exit=0, stdout begins "Usage: qwen [options] [command]", stderr empty (prompt never runs)
node packages/cli/dist/index.js mcp add probe-srv node /tmp/server.js --help
-> exit=0, stdout "Usage: qwen mcp add [options] <name> <commandOrUrl> [args...]", stderr empty
-> no server config written (find/grep -rl probe-srv over the throwaway HOME: nothing)
Two asks, both bookkeeping rather than code. Reword the claim to name the siblings and why they are out of scope here — for example that sessions answer is fenced because its tail is answered verbatim by a script, while the default command's [query..] positional and mcp add's [args...] tail have the same shape and are left to the argv scan consolidation #11065 owns. Please do not extend the fence to $0 [query..]: that is the CLI's primary interactive entry, pre-existing and untouched by this diff. Separately, qualify the closure — say that this PR fixes the sessions answer half of #11193, and either file a follow-up for the mcp add fast-path swallow or ask a maintainer whether #11193 should stay open until that row is decided.
One premise any follow-up must respect: the -v half of the mcp add surface is intentional and must not be "fixed" — cli.ts:340 lists mcp add name cmd server.js -v among the deliberate base-parity intercepts verified by A/B probes, so a follow-up must leave that row intercepting rather than demoting it to the full parser.
中文说明
这段 docstring 的“唯一性”断言正是把整个栅栏限定在单一 argv 前缀上的那句话，而本仓库里有两个同级命令与它矛盾。默认命令的可变 prompt positional（DEFAULT_COMMAND = '$0 [query..]'，config/top-level-options.ts:79）是同样的任意文本尾部形状且未被栅栏化：在构建出的 CLI 上，please --help me 会打印顶层用法块并以 exit 0 退出、stderr 为空，prompt 根本没有运行——正是这段注释只归给 sessions answer 的那种静默吞没。而 mcp add <name> <commandOrUrl> [args...]（commands/mcp/add.ts:198）有同样的尾部，cli.ts:452 的 hasFlag(argv.slice(2), '--help', '-h') 会对它触发。
代价有两方面：读到这句注释的维护者会以为没有别的命令拥有任意文本尾部，于是这两条孪生路径会继续吞没而无人去找；而 mcp add 这一行是今天就存在的静默 exit-0 实例，一个不加限定的 Fixes #11193 会把 issue 关掉却没有任何东西跟踪它——#11065 处于 open、priority/P3、status/blocked，且负责的是扫描整合而非这个行为。issue #11193 自己的 triage 记录已经点名了这一行。这一点在 PR 上尚未被讨论过——对现有四条 PR 评论 grep -i "mcp add" 命中数为零。
证据为在被审 commit 上实际运行两个同级形状（输出块见上方英文部分）。
两点请求，都属于记账而非代码：把这句断言改写为点名两个同级形状并说明为何此处不在范围内；请不要把栅栏扩展到 $0 [query..]，那是 CLI 的主要交互入口、既有且未被本 diff 触及。另外请限定关闭语义——说明本 PR 修复的是 #11193 的 sessions answer 那一半，并为 mcp add 快路径吞没另开 follow-up，或请维护者决定 #11193 是否应保持 open。
任何 follow-up 都必须尊重一个前提：mcp add 的 -v 那一半是有意的、不得“修复”——cli.ts:340 把 mcp add name cmd server.js -v 列为经 A/B 探测确认的、刻意的基线对齐拦截。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| return argv as string[]; | ||
| } | ||
| const session = argv[2]; | ||
| if (session === undefined || session.startsWith('-')) { |
There was a problem hiding this comment.
[Suggestion] R1-13: This guard bails on any dash-leading token in the id slot, so a boolean global option placed between answer and a real session id disables the fence at both sites — a third bypass placement that the docstring's carve-out list does not mention and no test covers.
qwen sessions answer --debug 0f8e1c42 please --version now (or with -d) is a valid, deliverable invocation: --debug is a root boolean global (config.ts:611, alias d), it is not in BASE_VALUE_FLAGS (cli.ts:108-119), and yargs accepts globals anywhere in the argv — verified against real yargs with a model of #10949's answerCommand, where the handler ran and bound session: "0f8e1c42", text: ["please","go"]. But argv[2].startsWith('-') is true, so the helper returns the array untouched at both cli.ts:333 and config.ts:590. At the bootstrap layer hasVersionToken then reaches the payload's --version and the route becomes 'version', printing the version and exiting 0 with nothing on stderr. At the parse layer the real parseArguments() prints the qwen sessions usage block and exits 0 — while this PR's own new test pins exit 1 for the identical payload behind a positional id (config.test.ts:346-349). So two spellings of one invocation diverge in exit semantics, and once #10949 lands the flag-shifted spelling delivers a silent success with the session still blocked.
The docstring argues only two adjacent states — the leading-global shape qwen --debug sessions answer ... deferred to #11065 (lines 30-32), and "<session> must be a real positional" for the no-id shapes (lines 34-37) — so this placement is not a documented design decision, and it contradicts the file's own claim at lines 21-23 that inserting a single -- right after the session id "closes every one of those paths at once".
Witness — probes against the real helper and the real resolveBootstrapRoute / parseArguments at this commit:
PROBE|A positional id (the shape the PR fixes) |fenceInserted=true |route=default
PROBE|B boolean global --debug before the id |fenceInserted=false|route=version (same for C: short global -d)
PROBE_BYPASS_DEBUG|err="process.exit unexpectedly called with \"0\""|stdoutHead="qwen sessions\n\nManage Qwen Code sessions..."
YGPROBE|Q1 global --debug before the id|outcome=parsed|seen={"handlerRan":true,"session":"0f8e1c42","text":["please","go"],"debug":true}
Either resolve the id slot instead of requiring a bare positional — skip dash-leading tokens using cli.ts's existing at-most-one-value-slot model and insert -- after the first non-dash token — or, if this residual stays deferred to #11065 like the leading-global case, say so in the carve-out list at lines 36-38 and pin it with a characterization test so the bypass is visible rather than silent. Note that the scanning fix is a small refactor rather than a local edit: VALUE_FLAGS and skipOptionValues are not exported from cli.ts today (cli.ts:85, :182). Please also coordinate with R1-16, whose fix edits this same guard line.
A scanning fix must reuse the existing one-token value-slot model rather than a greedy one — cli.ts:182's skipOptionValues carries the reason in its own comment: "At most one token: ... Consuming greedily would swallow a command token sitting after the values and misfire the top-level help fast path on it." Boolean globals such as --debug consume no value slot, so 0f8e1c42 must not be swallowed after them.
Please pin the current behaviour in cli.test.ts or session-answer-argv.test.ts with ['sessions','answer','--debug','0f8e1c42','please','--version','now'], then prove the pin by applying the wider id-slot resolution and confirming the expectation flips from unchanged/'version' to fenced/'default', and that removing the new scan reddens it. No assertion in the diff covers a dash-leading token in slot 2 other than the two "no id at all" shapes at session-answer-argv.test.ts:96-108.
One correction to the original report, from verification: the --session <id> spelling is not corroborated — against real yargs with a positional declared via .positional('session', ...), ['sessions','answer','--session','0f8e1c42','please','go'] binds session: "please" rather than re-binding the id, so this finding rests on the --debug/-d half alone.
中文说明
这个守卫会对 id 槽位中任何以短横开头的 token 提前返回，因此把一个布尔全局选项放在 answer 与真实 session id 之间，会在两个插入点同时让栅栏失效——这是第三种绕过位置，docstring 的 carve-out 列表没有提到，也没有任何测试覆盖。
qwen sessions answer --debug 0f8e1c42 please --version now（或 -d）是合法且可送达的调用：--debug 是根级布尔全局选项（config.ts:611，别名 d），不在 BASE_VALUE_FLAGS（cli.ts:108-119）里，而 yargs 允许全局选项出现在 argv 任意位置——已用真实 yargs 加上 #10949 answerCommand 的模型验证，handler 确实运行并绑定 session: "0f8e1c42"、text: ["please","go"]。但 argv[2].startsWith('-') 为真，于是 helper 在 cli.ts:333 与 config.ts:590 两处都原样返回数组。bootstrap 层随后 hasVersionToken 触及负载里的 --version，路由变成 'version'，打印版本号并 exit 0、stderr 为空；解析层则由真实 parseArguments() 打印 qwen sessions 用法块并 exit 0——而本 PR 自己新增的测试对同一负载（positional id 形式）钉住的是 exit 1（config.test.ts:346-349）。于是同一调用的两种拼写在退出语义上分叉，一旦 #10949 落地，flag 前置拼写就会在会话仍阻塞的情况下返回静默成功。
docstring 只论证了两个相邻状态——前置全局选项形状延后给 #11065（第 30-32 行），以及无 id 形状的“<session> 必须是真正的 positional”（第 34-37 行）——因此这个位置不是被记录下来的设计决定，而且与文件自身在第 21-23 行的说法矛盾。
证据为在被审 commit 上对真实 helper 与真实 resolveBootstrapRoute / parseArguments 的探测（输出块见上方英文部分）。
要么解析 id 槽位而不要求它是裸 positional——用 cli.ts 已有的“至多一个值槽”模型跳过短横开头的 token，并在第一个非短横 token 之后插入 --；要么如果这个残留像前置全局选项那样继续延后给 #11065，就在第 36-38 行的 carve-out 列表里写明，并用一个 characterization 测试钉住它，使这个绕过是可见的而非静默的。请注意扫描式修复是一次小重构而非局部编辑：VALUE_FLAGS 与 skipOptionValues 目前没有从 cli.ts 导出（cli.ts:85、:182）。也请与 R1-16 协调，其修复会改同一行守卫。
扫描式修复必须复用已有的单 token 值槽模型而不是贪婪模型——cli.ts:182 的 skipOptionValues 在其注释里给出了理由；--debug 这类布尔全局选项不消耗值槽，因此 0f8e1c42 不能在其后被吞掉。
请在 cli.test.ts 或 session-answer-argv.test.ts 中钉住当前行为，然后通过套用更宽的 id 槽解析、确认期望从“原样/'version'”翻转为“栅栏化/'default'”，并确认移除新扫描会使其变红，来证明这个钉子有效。
来自验证的一处更正：--session <id> 这一拼写未被证实，因此本条只依赖 --debug/-d 那一半。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| return argv as string[]; | ||
| } | ||
| // The bare-`--help` carve-out: `answer <session> --help` shows help. | ||
| if (argv.length === 4 && (argv[3] === '--help' || argv[3] === '-h')) { |
There was a problem hiding this comment.
[Suggestion] R1-5-1 (location 1 of 2): The single-token --version / -v payload is fenced by this code but pinned by no test at any layer, while its --help / -h sibling on this very line is pinned in three. That asymmetry makes the natural symmetry edit invisible: widening this carve-out to argv[3] === '--version' || argv[3] === '-v' — the obvious next step, since the -h/--help pair is already carved out right here — makes qwen sessions answer <id> --version take hasVersionToken, route 'version', and print the version with exit 0 while the one-token answer is silently dropped. That is #11193's third repro row, and every test still passes.
It is not an equivalent mutant: the fenced and carved-out outcomes differ observably for that input ('default' versus 'version'). Every version-token case in the new tests carries siblings or sits in the id slot — session-answer-argv.test.ts:66,75 use ['please','--version','now'], :83 a bare root ['--version'], :86 mcp remove victim -v help, :105-108 the id slot ['sessions','answer','-v']; cli.test.ts:162,175 use the three-token payload and a user-supplied --; config.test.ts has no version case at all.
Witness — mutant applied at this line, pristine PR test files:
INTACT helper -> ["sessions","answer","0f8e1c42","--","--version"] route "default" 497 tests green
MUTANT M1 helper -> ["sessions","answer","0f8e1c42","--version"] route "version" -> printBootstrapVersion()
Test Files 4 passed (4), Tests 497 passed (497) <- non-equivalent mutant, zero red
Please add, next to the bare---help case in session-answer-argv.test.ts, assertions that insertSessionAnswerSeparator(['sessions','answer','0f8e1c42','--version']) equals ['sessions','answer','0f8e1c42','--','--version'] and the same for '-v', plus one route-level line in cli.test.ts: expect(resolveBootstrapRoute(['sessions','answer','0f8e1c42','--version'])).toBe('default'). The route-level line matters because the parse layer cannot see this mutant at all — sessions' builder calls .version(false), so an unfenced --version is a strict unknown-argument exit 1 either way. The harm lives entirely at the bootstrap layer.
Please do not "fix" the asymmetry instead of pinning it. The version intercept is deliberately fail-closed for every shape, per cli.ts:344-347: "Printing the version is side-effect-free, while demoting to the full parser EXECUTES subcommands (observed: mcp remove victim -v help deleted the server and its OAuth creds on the full parser) — so the fail-closed direction is to intercept." Widening this carve-out would re-open the payload to that intercept.
Prove the new assertions by widening the carve-out to version tokens and confirming both the helper assertions and the cli.test.ts route assertion go red.
中文说明
（两处同类问题中的第 1 处）单 token 的 --version / -v 负载被这段代码栅栏化了，但在任何一层都没有测试钉住它；而就在这一行的 --help / -h 兄弟形状却在三处被钉住。这个不对称使得最自然的对称化编辑变得不可见：把该 carve-out 扩展为 argv[3] === '--version' || argv[3] === '-v'（既然 -h/--help 已经在这里被 carve out，这是显而易见的下一步）会让 qwen sessions answer <id> --version 走到 hasVersionToken、路由 'version'，打印版本号并 exit 0，同时单 token 的回答被静默丢弃。那正是 #11193 复现表的第三行，而所有测试仍然通过。
它不是等价变异体：对该输入而言，栅栏化与 carve-out 两种结果在可观测上不同（'default' 对 'version'）。新测试中每个 version-token 用例要么带有兄弟 token、要么位于 id 槽位；config.test.ts 则完全没有 version 用例。
证据为在这一行套用变异体、并保持 PR 测试文件原样的运行结果（输出块见上方英文部分）。
请在 session-answer-argv.test.ts 的 bare---help 用例旁边补上断言，并在 cli.test.ts 补一行路由级断言。路由级那一行很关键，因为解析层完全看不到这个变异体——sessions 的 builder 调用了 .version(false)，所以未栅栏化的 --version 在两种情况下都是 strict 的 unknown-argument exit 1；危害完全存在于 bootstrap 层。
请不要用“修掉这个不对称”来代替钉住它。version 拦截对每种形状都是刻意 fail-closed 的，理由见 cli.ts:344-347；扩展这个 carve-out 会把负载重新暴露给该拦截。
请通过把 carve-out 扩展到 version token、确认 helper 断言与 cli.test.ts 路由断言都变红，来证明新断言有效。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| if (argv.length < 4) { | ||
| return argv as string[]; | ||
| } | ||
| if (argv[3] === '--') { |
There was a problem hiding this comment.
[Suggestion] R1-5-2 (location 2 of 2): This user-supplied-separator guard is position-sensitive — only index 3 counts — but every -- in the three new test files sits at index 3. So relaxing it to a whole-argv scan is invisible to the suite, and the relaxation re-opens the exit-0 swallow this PR exists to close.
The trigger is an answer that quotes CLI flags and a separator, for example qwen sessions answer 0f8e1c42 use --help -- or -h. With the guard relaxed to argv.includes('--'), that argv is returned unfenced, and the real parseArguments() then prints the help block and exits 0 instead of failing loudly. The route layer cannot catch it: resolveBootstrapRoute returns 'default' for both the fenced and unfenced forms (firstPositionalArg is 'sessions', so the help fast path is off, and hasVersionToken/hasFlag find no exact token), which makes this regression parse-layer-only. A population sweep puts the number of tests that distinguish this guard's position-sensitivity at zero — the only -- inputs are session-answer-argv.test.ts:142 and cli.test.ts:173, both at index 3, and config.test.ts's two new argvs contain no -- at all.
Witness — mutant applied at this line, pristine PR test files, and the parse-layer consequence measured through the real parseArguments() rather than a yargs replica:
ARM PROOF trigger ["sessions","answer","0f8e1c42","use","--help","--","or","-h"] returned UNFENCED
SURVIVAL Test Files 3 passed (3), Tests 496 passed (496) <- identical to the intact baseline
PARSE LAYER intact: exitMessage "process.exit unexpectedly called with \"1\"" printedHelpBlock false
mutant: exitMessage "process.exit unexpectedly called with \"0\"" printedHelpBlock true
Please add a case with -- past index 3: in session-answer-argv.test.ts assert that insertSessionAnswerSeparator(['sessions','answer','0f8e1c42','use','--help','--','or','-h']) equals ['sessions','answer','0f8e1c42','--','use','--help','--','or','-h'], and mirror it in config.test.ts's does not let a sessions-answer payload quote help into the help intercept loop. The mirror is what pins the parse layer, which the helper case alone cannot reach.
The rationale at lines 42-43 constrains the new case: "A user-supplied separator is never doubled: the tail recovery splices only the first --, so a second one would leak into the answer text." So session-answer-argv.test.ts:136-146 must keep asserting that a -- at index 3 returns the argv unchanged.
Prove the new assertions by applying the argv.includes('--') relaxation and confirming both go red — the proposed cases were measured to do exactly that, with the existing index-3 case still passing.
中文说明
（两处同类问题中的第 2 处）这个“用户自供分隔符”守卫是位置敏感的——只有索引 3 算数——但三个新测试文件里的每个 -- 都恰好位于索引 3。因此把它放宽成对整个 argv 的扫描对测试套件是不可见的，而这个放宽会重新打开本 PR 要关闭的 exit-0 吞没。
触发形状是一段同时引用了 CLI flag 与分隔符的回答，例如 qwen sessions answer 0f8e1c42 use --help -- or -h。把守卫放宽为 argv.includes('--') 后，该 argv 会被原样返回（未栅栏化），真实 parseArguments() 随后打印帮助块并 exit 0，而不是响亮地失败。路由层抓不到它：对栅栏化与未栅栏化两种形式 resolveBootstrapRoute 都返回 'default'，所以这个回归只存在于解析层。穷举统计显示能区分该守卫位置敏感性的测试数量为零——唯一的 -- 输入是 session-answer-argv.test.ts:142 与 cli.test.ts:173，都在索引 3，而 config.test.ts 的两个新 argv 完全不含 --。
证据为在这一行套用变异体、保持 PR 测试文件原样，并通过真实 parseArguments()（而非 yargs 复制品）测量解析层后果的运行结果（输出块见上方英文部分）。
请补一个 -- 位于索引 3 之后的用例：在 session-answer-argv.test.ts 中断言插入结果，并在 config.test.ts 的 does not let a sessions-answer payload quote help into the help intercept 循环里镜像一份。镜像那一份才是钉住解析层的关键，仅靠 helper 用例够不到。
第 42-43 行的理由约束了新用例：“用户自供的分隔符绝不会被重复插入：尾部恢复只会剥掉第一个 --，因此第二个会泄漏进回答文本。”所以 session-answer-argv.test.ts:136-146 必须继续断言索引 3 处的 -- 会原样返回 argv。
请通过套用 argv.includes('--') 放宽、确认两条新断言都变红来证明它们有效（已实测如此，且既有的索引 3 用例仍然通过）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
2 participants
What this PR does
This PR stops the bootstrap help/version intercepts from silently swallowing the free-text payload of
qwen sessions answer <session> <answer...>. When the answer text quotes--help,-h,--version/-v, or ends in a barehelp, the CLI used to print the usage block or the version and exit 0, so a script driving a background session (qwen sessions answer <id> ... || fail) read success while the answer was never delivered.The fix inserts a single
--separator between the session id and the answer payload, in the two places that rewrite raw argv before yargs parses it: the bootstrap route scan (resolveBootstrapRouteincli.ts) andparseArguments()inconfig.ts(next to the existing entry-point-token rewrite). After the separator, every later token is positional data by construction — to thehasVersionToken/hasFlagbootstrap scans, which stop at--, and to the yargs parse, which cannot turn post---tokens into flags or command matches. A new shared helper (insertSessionAnswerSeparator) implements the insertion once and is applied identically at both layers.The carve-outs the issue demands are preserved and pinned by tests: a payload that is exactly
--help(or-h) still shows the command's help instead of being delivered as the literal text--help;qwen sessions answer --help(no id) still routes to help; a user-supplied--is never doubled; and the fail-closed version intercept for every other command (qwen sessions list -v,qwen mcp remove x -v help, ...) is untouched —sessions-prefixed argv with a realanswer <id>payload is the only shape that changes.Note on timing: the
sessions answersubcommand itself lands with #10949, which does not touchcli.tsorconfig.ts. This PR fixes the pre-existing bootstrap/root-argv half the issue was filed against, so it can merge independently and in any order. On today'smain(without #10949) the three swallow shapes now fail loudly with the strict unknown-command error and exit 1 instead of printing help/version and exiting 0; once #10949 lands, the same argv delivers the answer verbatim — itsrawAnswerTailcuts the text from the raw args and splices the first--back out, so the injected separator is scaffolding that never reaches the answer text.Why it's needed
Issue #11193: the bootstrap layer runs its argv scans before any subcommand parses, and
sessions answeris the first command whose positional argument is arbitrary user prose. Any answer that happens to contain a help/version token was consumed by an intercept that prints and exits 0 — the failure path is loud (--debugon a nonexistent flag exits 1 on stderr) while the swallow path is silent, which is the worst combination for scripted use. The issue's guardrail is that the intercepts must stay for every other command (demoting them executes subcommands —qwen mcp remove victim -v helpwould delete the server), so the fix carves the payload out at the argv layer instead of weakening any scan, exactly the "insert a single--aftersessions answer <session>" shape the issue proposes.Reviewer Test Plan
How to verify
Regression tests go through the real entry routes the issue names —
resolveBootstrapRouteandparseArguments— not just internal helpers:packages/cli/src/utils/session-answer-argv.test.ts— the insertion matrix: the three reproduction shapes get fenced; other commands, missing/flag-like session ids, a bare--helppayload, an existing--, and an id with no payload are left untouched.packages/cli/src/cli.test.ts(resolveBootstrapRoute) — the three reproduction argvs route to'default'instead of'version'/being swallowed, whilesessions list -v,mcp remove victim -v help,sessions answer --helpand the bare---helppayload keep their intercept/help behavior.packages/cli/src/config/config.test.ts(parseArguments, real parse with mockedprocess.argv) — the--help-quoting and bare-helpanswers now take the strict unknown-command failure (exit 1) instead of the help intercept (exit 0); the bare--helppayload still shows help and exits 0.Against a built CLI (
npm run build, thennode packages/cli/dist/index.js ... </dev/null), each row of the issue's reproduction table:qwen sessions answer <id> please --help meplease --help meonce #10949 lands)qwen sessions answer <id> yes please helpyes please helponce #10949 lands)qwen sessions answer <id> please --version nowplease --version nowonce #10949 lands)qwen sessions answer <id> --helpEvidence (Before & After)
Non-UI change; verification is the test suites and the built-CLI probe above. After the fix, all eight probed shapes behave as listed (three swallow shapes loud-exit 1, both
--helpcarve-outs still print help with exit 0,sessions list -vstill prints the version with exit 0, and a user-supplied--behaves identically to before).Tested on
Environment (optional)
npm ci+npm run buildat the repo root; unit/entry tests viaLC_ALL=en_US.UTF-8 npx vitest runinpackages/cli; CLI probes vianode packages/cli/dist/index.js ... </dev/null.Risk & Scope
sessionsas the first token,answeras the second, a non-flag session id third). A flag-shifted shape (qwen --debug sessions answer ...) is deliberately not fenced — recognizing it needs a value-slot scan, which is exactly the class of hand-maintained argv scans issue refactor(cli): consolidate the hand-maintained value-slot-skipping argv scans #11065 owns consolidating; until then those invocations keep the existing escapes (--,=-form tokens, or a version token in a base value-flag slot).sessions answercommand (it does not exist onmainyet — it lands with feat(cli): see, answer and stop a background session #10949, whoserawAnswerTailandforgetInheritedOptionsare designed to consume the fenced payload verbatim); flag-shifted invocations; interactions with refactor(cli): consolidate the hand-maintained value-slot-skipping argv scans #11065's upcoming scan consolidation.sessions answer <id> <payload>argv where the payload quotes help/version tokens: silent exit 0 becomes a loud exit 1 today, and correct delivery once feat(cli): see, answer and stop a background session #10949 merges. A payload that is exactly--help/-hintentionally keeps showing help.Linked Issues
Fixes #11193
中文说明
这个 PR 做了什么
本 PR 修复了 bootstrap 层的 help/version 拦截静默吞掉
qwen sessions answer <session> <answer...>自由文本负载的问题。此前，当回答文本中引用了--help、-h、--version/-v，或以单独的help单词结尾时，CLI 会打印用法说明或版本号并以 exit 0 退出——驱动后台会话的脚本（qwen sessions answer <id> ... || fail）会读到"成功"，而回答根本没有送达。修复方式是在 session id 与回答负载之间插入一个
--分隔符，插入位置是 yargs 解析前重写 raw argv 的两处：bootstrap 路由扫描（cli.ts的resolveBootstrapRoute）和config.ts的parseArguments()（紧挨现有的 entry-point-token 重写）。分隔符之后的每个 token 按构造就是 positional data——对遇到--即停止的hasVersionToken/hasFlagbootstrap 扫描如此，对无法把--之后的 token 变成 flag 或 command 匹配的 yargs 解析也如此。新增的共享 helper（insertSessionAnswerSeparator）只实现一次插入逻辑，两层以完全相同的方式调用。issue 要求保留的 carve-out 均已保留并由测试钉死：恰为
--help（或-h）的负载仍然显示命令帮助而不是把字面文本--help当回答送达；qwen sessions answer --help（不带 id）仍然路由到帮助；用户自己提供的--不会被重复插入；对其他所有命令的 fail-closed version 拦截（qwen sessions list -v、qwen mcp remove x -v help等）原封不动——唯一行为变化的形状是带真实answer <id>负载的sessions前缀 argv。关于时序的说明：
sessions answer子命令本身随 #10949 落地，而那个 PR 不改动cli.ts和config.ts。本 PR 修复的是 issue 所指向的、预先存在的 bootstrap/root-argv 半边问题，因此可以独立合并且合并顺序无关。在当前main上（还没有 #10949），三种吞没形状现在会以 strict 的 unknown-command 错误响亮地 exit 1，而不是打印 help/version 后 exit 0；一旦 #10949 落地，同样的 argv 会把回答原样送达——它的rawAnswerTail从原始 args 截取文本并把第一个--剥掉，所以注入的分隔符只是脚手架，永远不会进入回答文本。为什么需要它
Issue #11193：bootstrap 层在任何子命令解析之前运行 argv 扫描，而
sessions answer是第一个位置参数为任意用户文本的命令。任何恰好包含 help/version token 的回答都会被某个拦截消费掉——打印并 exit 0。失败路径是响亮的（不存在的 flag 上--debug会在 stderr 上 exit 1），而吞没路径是静默的，这对脚本化使用是最坏的组合。issue 的护栏是：对其他所有命令必须保留拦截（降级会执行子命令——qwen mcp remove victim -v help会删掉 server），因此修复选择在 argv 层把负载剥出来，而不是削弱任何扫描——正是 issue 提出的"在sessions answer <session>后插入单个--"方案。审阅者测试计划
如何验证
回归测试走 issue 点名 的真实入口路由——
resolveBootstrapRoute和parseArguments——而不只是内部 helper：packages/cli/src/utils/session-answer-argv.test.ts——插入矩阵：三个复现形状被栅栏隔离；其他命令、缺失/flag 形式的 session id、恰为--help的负载、已存在的--、无负载的 id 均保持原样。packages/cli/src/cli.test.ts（resolveBootstrapRoute）——三个复现 argv 路由到'default'而不是'version'/被吞；而sessions list -v、mcp remove victim -v help、sessions answer --help和恰为--help的负载保持其拦截/帮助行为。packages/cli/src/config/config.test.ts（parseArguments，mockprocess.argv的真实解析）——引用--help的回答和以裸help结尾的回答现在走 strict 的 unknown-command 失败（exit 1）而不是 help 拦截（exit 0）；恰为--help的负载仍然显示帮助并 exit 0。针对构建出的 CLI（
npm run build后node packages/cli/dist/index.js ... </dev/null），issue 复现表的每一行：qwen sessions answer <id> please --help meplease --help me）qwen sessions answer <id> yes please helpyes please help）qwen sessions answer <id> please --version nowplease --version now）qwen sessions answer <id> --help证据（前后对比）
非 UI 改动；验证即上述测试套件与构建 CLI 的探测。修复后，八个探测形状全部符合预期（三个吞没形状响亮 exit 1，两个
--helpcarve-out 仍打印帮助且 exit 0，sessions list -v仍打印版本且 exit 0，用户自供的--行为与之前一致）。测试环境
环境（可选）
仓库根
npm ci+npm run build；packages/cli下以LC_ALL=en_US.UTF-8 npx vitest run跑单元/入口测试；node packages/cli/dist/index.js ... </dev/null探测 CLI。风险与范围
sessions、第二个是answer、第三个是非 flag 的 session id）。flag 前置形状（qwen --debug sessions answer ...）刻意不做栅栏——识别它需要值槽扫描，而那正是 issue refactor(cli): consolidate the hand-maintained value-slot-skipping argv scans #11065 负责整合的手工维护 argv 扫描类别；在那之前，这类调用保有既有的逃生通道（--、=形式 token、或位于基础值 flag 槽位中的 version token）。sessions answer命令的端到端送达（它还不在main上——随 feat(cli): see, answer and stop a background session #10949 落地，其rawAnswerTail与forgetInheritedOptions正是为原样消费栅栏化负载而设计）；flag 前置调用；与 refactor(cli): consolidate the hand-maintained value-slot-skipping argv scans #11065 即将进行的扫描整合的交互。sessions answer <id> <payload>argv 上：今天的静默 exit 0 变为响亮的 exit 1，feat(cli): see, answer and stop a background session #10949 合并后变为正确送达。恰为--help/-h的负载有意保留显示帮助。关联 Issue
Fixes #11193