Skip to content

feat(cli): select an output style via general.outputStyle or --output-style - #10283

Open
qqqys wants to merge 8 commits into
QwenLM:mainfrom
qqqys:feat/output-style-setting
Open

feat(cli): select an output style via general.outputStyle or --output-style#10283
qqqys wants to merge 8 commits into
QwenLM:mainfrom
qqqys:feat/output-style-setting

Conversation

@qqqys

@qqqys qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This adds the first way to actually pick one of the output styles that #9565 shipped: a general.outputStyle setting and a per-run --output-style <name> flag that overrides it. Names resolve case-insensitively against the built-in styles — Concise, Proactive, Explanatory, Learning — so "outputStyle": "concise" in settings.json and --output-style Concise on the command line select the same thing. An unset or empty value, or the literal default, means no style; --output-style default is therefore also the way to switch a style off for one run when the setting names one.

An unknown name is reported, not fatal. The session prints WARNING: Unknown output style "Verbose" (from general.outputStyle); using the default style. Available styles: Concise, Proactive, Explanatory, Learning. and starts with the default style, so a typo in settings.json never locks anyone out of the CLI. The warning names whichever source supplied the bad value, and the same text goes to the debug log so it is still discoverable after the terminal scrollback is gone — the same treatment the TLS-verification warning already gets in loadCliConfig.

The setting is declared showInDialog: false for now: the /settings picker and a /output-style command that applies a change mid-session come in the next slice, where the live refreshSystemInstruction() hand-off can be reviewed on its own. On this PR a style is chosen once at startup and holds for the session, which is exactly what the headless -p path needs. The settings reference and the headless guide document the setting, the flag, and the fact that Learning — which hands the user a piece of code and waits — is skipped in headless runs, as getCoreSystemPrompt already arranges.

Why it's needed

#9565 landed the styles and their prompt layering with no way to select one, so today they are unreachable: the only levers a user has for the agent's voice are still --append-system-prompt (per-invocation, undiscoverable) and QWEN_SYSTEM_MD (an all-or-nothing prompt replacement they must maintain by hand). A settings key gives the choice persistence and a scope (user or project); the flag gives scripts and one-off runs a way to override it without touching a file. Together they make the built-in styles usable end to end for headless and interactive sessions alike, and they are the substrate the interactive picker, custom style files, and the output_style field on system/init will all read from.

Resolving the name in loadCliConfig — rather than inside Config — keeps Config unaware of settings files and argv, which is how every other CLI-facing option (--append-system-prompt, general.preventSystemSleep) already reaches it.

Reviewer Test Plan

How to verify

From packages/cli, run npx vitest run packages/cli/src/config/config.test.ts packages/cli/src/config/settingsSchema.test.ts from the repo root (or the same paths relative to packages/cli) — 418 tests pass, 13 of them new. The output style block asserts: no style by default; general.outputStyle: "concise" resolves to the built-in Concise; --output-style Explanatory beats a setting of Concise; --output-style default yields no style even when the setting names one, and prints nothing; a whitespace-only value does the same; an unknown name leaves the style unset and prints a warning that names the source (general.outputStyle vs --output-style) and lists the four available styles; a non-string setting value (number, boolean, object, array) and a repeated --output-style flag both warn and start with the default style instead of throwing. settingsSchema.test.ts pins requiresRestart: true. parseArguments has a case for the new flag.

Mutation-verified, each run against the current tree: dropping the typeof raw !== 'string' guard reddens 5 tests with raw.trim is not a function; dropping || name.toLowerCase() === 'default' reddens 2 on the "no warning" assertion; dropping .trim() reddens the whitespace test; flipping requiresRestart back to false reddens the schema test.

To see it end to end (not run here — unit tests only): qwen -p "Explain what this repo does" --output-style Concise answers without preamble; qwen -p "..." --output-style Verbose prints the warning on stderr and then answers in the default voice; QWEN_WRITE_SYSTEM_MD=1 qwen -p hi --output-style Concise still writes a base prompt with no style section, as #9565 specified.

npm run typecheck --workspace @qwen-code/qwen-code is clean for the changed files (the four CliArgs literals in gemini.test.tsx gained the new field, which is what makes it required-but-undefined there). eslint and prettier --check were run over every changed file; settingsSchema.test.ts, settings.test.ts, config-command.test.ts (258 tests) and gemini.test.tsx (76 tests) pass.

Evidence (Before & After)

N/A for the TUI — the setting is not yet shown in /settings. Before: no --output-style flag exists and general.outputStyle is ignored. After: qwen --help lists --output-style, and the flag or setting selects a built-in style for the session.

Tested on

OS Status
🍏 macOS ⚠️ not tested
🪟 Windows ⚠️ not tested
🐧 Linux ✅ tested

Environment (optional)

Linux, Node 22, unit tests.

Risk & Scope

  • Main risk or tradeoff: an unknown style degrades to the default with a warning rather than failing fast. That is deliberate — a stale or mistyped value in a shared project settings.json should not stop every contributor's CLI from starting — but it does mean a misspelled flag is easy to miss in a script that discards stderr.
  • Not validated / out of scope: no /settings entry or /output-style command yet, so a style cannot be changed mid-session; custom .qwen/output-styles/*.md files, extension-provided styles, the per-turn reminder (feat(core): remind the model of the active output style every turn #10282), and the output_style field on system/init are separate slices. The style prompts themselves have not been evaluated against a live model; the end-to-end commands above are what a reviewer can run, not something exercised here beyond unit tests.
  • Breaking changes / migration notes: none. CliArgs gains a required outputStyle field, which affects only code that builds the whole struct by hand (the four test literals updated here).

Linked Issues

Follows #9565; independent of #10282.

中文说明

这个 PR 做了什么

本 PR 提供了第一个真正能选中 #9565 所引入的输出风格的入口:一个 general.outputStyle 设置项,以及一个覆盖它的单次运行参数 --output-style <name>。名称对内置风格 —— ConciseProactiveExplanatoryLearning —— 大小写不敏感地解析,所以 settings.json 里的 "outputStyle": "concise" 和命令行上的 --output-style Concise 选中的是同一个东西。未设置、空值或字面量 default 表示不使用风格;因此当设置项指定了某个风格时,--output-style default 也是在单次运行中关闭它的方式。

未知的名称只会被报告,不会致命。会话会打印 WARNING: Unknown output style "Verbose" (from general.outputStyle); using the default style. Available styles: Concise, Proactive, Explanatory, Learning. 然后以默认风格启动,所以 settings.json 里的一个拼写错误绝不会把任何人挡在 CLI 之外。警告会指出是哪个来源提供了错误的值,同样的文本也会写入调试日志,这样终端滚动缓冲区消失后仍然可以找到 —— 这与 loadCliConfig 里 TLS 校验警告已有的处理方式一致。

设置项目前声明为 showInDialog: false/settings 中的选择器和能在会话中途切换的 /output-style 命令会在下一个切片中到来,届时实时的 refreshSystemInstruction() 交接可以被单独评审。在本 PR 中,风格在启动时选定一次并在整个会话中保持,这正是 headless -p 路径所需要的。settings 参考文档和 headless 指南记录了该设置项、参数,以及 Learning(它把一段代码交给用户并等待)在 headless 运行中会被跳过这一点 —— getCoreSystemPrompt 已经如此安排。

为什么需要它

#9565 落地了风格及其提示词分层,却没有任何选择方式,所以今天它们是不可达的:用户能控制 agent 语气的手段仍然只有 --append-system-prompt(每次调用都要带、无从发现)和 QWEN_SYSTEM_MD(对提示词的全有或全无式替换,还得自己手工维护)。一个 settings 键让这个选择得以持久化并具有作用域(用户级或项目级);参数则让脚本和一次性运行无需改文件就能覆盖它。二者结合使内置风格在 headless 和交互式会话中都端到端可用,也是交互式选择器、自定义风格文件和 system/init 上的 output_style 字段将来共同读取的基础。

loadCliConfig 中解析名称 —— 而不是在 Config 内部 —— 让 Config 不必知道 settings 文件和 argv 的存在,这正是其他所有面向 CLI 的选项(--append-system-promptgeneral.preventSystemSleep)到达它的方式。

评审者测试计划

如何验证

在仓库根目录运行 npx vitest run packages/cli/src/config/config.test.ts packages/cli/src/config/settingsSchema.test.ts(或在 packages/cli 下使用相对路径)—— 418 个测试通过,其中 13 个是新增的。output style 块断言:默认没有风格;general.outputStyle: "concise" 解析为内置的 Concise--output-style Explanatory 优先于设置项中的 Concise;即便设置项指定了风格,--output-style default 也得到无风格且不打印任何内容;纯空白值同理;未知名称使风格保持未设置,并打印一条指明来源(general.outputStyle--output-style)且列出四个可用风格的警告;非字符串的设置值(数字、布尔、对象、数组)以及重复的 --output-style 参数都会告警并以默认风格启动,而不是抛异常。settingsSchema.test.ts 钉住了 requiresRestart: trueparseArguments 有一个针对新参数的用例。

变异验证(均在当前代码上运行):移除 typeof raw !== 'string' 守卫会让 5 个测试变红并报 raw.trim is not a function;移除 || name.toLowerCase() === 'default' 会让 2 个测试在"无警告"断言上变红;移除 .trim() 会让空白值测试变红;把 requiresRestart 改回 false 会让 schema 测试变红。

端到端验证(本 PR 未实际运行,仅单元测试):qwen -p "Explain what this repo does" --output-style Concise 会不带铺垫地直接作答;qwen -p "..." --output-style Verbose 会在 stderr 打印警告,然后以默认语气作答;QWEN_WRITE_SYSTEM_MD=1 qwen -p hi --output-style Concise 写出的仍是不含风格段落的 base 提示词,与 #9565 的规定一致。

npm run typecheck --workspace @qwen-code/qwen-code 对改动文件无错误(gemini.test.tsx 里四处 CliArgs 字面量补上了新字段,这正是该字段在那里必须存在但为 undefined 的原因)。对每个改动文件运行了 eslintprettier --checksettingsSchema.test.tssettings.test.tsconfig-command.test.ts(258 个测试)以及 gemini.test.tsx(76 个测试)全部通过。

证据(Before & After)

TUI 方面 N/A —— 该设置项尚未在 /settings 中显示。Before:不存在 --output-style 参数,general.outputStyle 被忽略。After:qwen --help 列出 --output-style,参数或设置项可以为会话选中一个内置风格。

测试环境

操作系统 状态
🍏 macOS ⚠️ 未测试
🪟 Windows ⚠️ 未测试
🐧 Linux ✅ 已测试

环境(可选)

Linux,Node 22,单元测试。

风险与影响范围

  • 主要风险或权衡:未知风格会带着警告降级为默认风格,而不是快速失败。这是有意为之 —— 共享的项目级 settings.json 里一个过期或拼错的值不应让每位贡献者的 CLI 都无法启动 —— 但这也意味着在丢弃 stderr 的脚本里,拼错的参数容易被忽略。
  • 未验证 / 不在范围内:尚无 /settings 条目或 /output-style 命令,所以风格不能在会话中途切换;自定义 .qwen/output-styles/*.md 文件、extension 提供的风格、每轮提醒(feat(core): remind the model of the active output style every turn #10282)以及 system/init 上的 output_style 字段是各自独立的切片。风格提示词文案本身尚未在真实模型上评估;上面的端到端命令是供评审者运行的,本 PR 除单元测试外没有实际执行过它们。
  • 破坏性改动 / 迁移说明:无。CliArgs 新增一个必填的 outputStyle 字段,只影响手工构造整个结构体的代码(本 PR 更新了四处测试字面量)。

关联 Issue

承接 #9565;与 #10282 相互独立。

…-style

Adds the first selection surface for the output styles QwenLM#9565 shipped:
a `general.outputStyle` setting and a per-run `--output-style` flag that
overrides it. Names resolve case-insensitively against the built-in
styles; unset, empty, or "default" means no style, and an unknown name
prints a warning and starts the session with the default style instead
of refusing to start.

Claude-Session: https://claude.ai/code/session_01Gk1gryVzWuW58zkBtoBvnM
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 0b34f32 did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 0b34f32 有 1 个 PR CI workflow 未以绿色完成,详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage查看 finalize 运行

@qqqys

qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 27, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: real and observed, not theoretical — #9565 merged the output-style prompt layer (Concise / Proactive / Explanatory / Learning) but shipped no way to pick one, so the styles are unreachable today. This PR is the missing selector. Verified #9565 is merged and its surface (BUILT_IN_OUTPUT_STYLES, getBuiltInOutputStyle, the Config outputStyle param, getMainSessionBaseSystemPrompt) all exists in the base.

Direction: aligned. Output styles are an established, actively maintained feature in the reference product — Claude Code's CHANGELOG records the built-in "Concise" style, then the /output-style command deprecated in favor of selection fixed at session start for prompt-cache stability, and later un-deprecated after community feedback. Choosing once at startup via a setting, with a per-run flag override, is exactly the direction that history converged on.

Size: touches core-protected paths (packages/cli/src/config/**). Production logic: 52 lines (config.ts +42, settingsSchema.ts +10); tests: 79 lines (config.test.ts +75, gemini.test.tsx +4); docs: 14 lines. feat-type and well under any threshold — no maintainer escalation needed. Every downstream consumer of the new value is pre-existing #9565 wiring: getMainSessionBaseSystemPrompt (client.ts), ArenaManager, and the Config constructor param.

Approach: the scope feels right for a first slice. Resolving the name in loadCliConfig follows the existing precedent (--append-system-prompt, general.preventSystemSleep), the unknown-name warns-but-doesn't-fail choice mirrors the TLS warning already in that file, and deferring the /settings picker and the per-turn reminder (#10282) to follow-up slices keeps this reviewable. No simpler path missed, and no drive-by changes — every line serves the stated goal.

Risk: no high-risk path matches; no elevated risk signals.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题: 真实且已观测到,不是理论问题 —— #9565 合并了输出风格提示词层(Concise / Proactive / Explanatory / Learning),却没有留下任何选择入口,所以这些风格目前不可达。本 PR 补上缺失的选择器。已验证 #9565 已合并,其接口(BUILT_IN_OUTPUT_STYLESgetBuiltInOutputStyleConfigoutputStyle 参数、getMainSessionBaseSystemPrompt)均存在于 base 分支。

方向: 对齐。输出风格在参考产品中是成熟且持续维护的功能 —— Claude Code 的 CHANGELOG 记录了内置 "Concise" 风格、随后弃用 /output-style 命令改为会话启动时固定选择(为了提示词缓存稳定性)、社区反馈后又恢复启用。通过设置项在启动时选定一次、用单次运行参数覆盖,正是那段演化最终收敛的方向。

规模: 触及核心保护路径(packages/cli/src/config/**)。生产逻辑 52 行(config.ts +42、settingsSchema.ts +10);测试 79 行(config.test.ts +75、gemini.test.tsx +4);文档 14 行。feat 类型且远低于任何阈值 —— 无需升级维护者关注。新值的所有下游消费者都是 #9565 已有的接线:getMainSessionBaseSystemPrompt(client.ts)、ArenaManager 以及 Config 构造参数。

方案: 作为第一个切片范围合理。在 loadCliConfig 中解析名称遵循既有先例(--append-system-promptgeneral.preventSystemSleep),未知名称"警告但不失败"的选择与同文件中已有的 TLS 警告处理一致,把 /settings 选择器和每轮提醒(#10282)留给后续切片也让本次评审保持聚焦。没有遗漏更简路径,也没有夹带改动 —— 每一行都服务于既定目标。

风险: 无高风险路径命中,无升级风险信号。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 0b34f321cf601e0c1a132a22c43ea276ba69dff3 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Code review

Reviewed the full diff (145 additions, 0 deletions, 6 files) against the base at the commit below. No blocking findings, no convention violations.

  • The implementation reuses the feat(core): add the output-style layer to the system prompt #9565 surface instead of duplicating it: getBuiltInOutputStyle / BUILT_IN_OUTPUT_STYLES come from core, and the resolved definition goes straight into the existing Config outputStyle param. The only new logic is resolveOutputStyle (precedence, default, unknown-name warning), which belongs in the CLI layer and mirrors the TLS-warning pattern already in loadCliConfig (debugLogger.warn + console.error('WARNING: …')).
  • I traced the whole chain statically: loadCliConfigConfig.outputStylegetMainSessionBaseSystemPrompt (client.ts) → getCoreSystemPromptapplyOutputStyle, and ArenaManager picks the style up too. This is not a dead switch.
  • The docs claims check out against the code: Learning is skipped in headless runs, a QWEN_SYSTEM_MD override ignores the style, and an unknown name warns with the source (--output-style vs general.outputStyle) named.
  • Tests cover the cases that matter: unset by default, case-insensitive resolution, flag-over-setting precedence, default as an opt-out, and unknown-name warnings attributed to the right source from both entry points. The new required CliArgs.outputStyle field is updated at all four hand-built literals in gemini.test.tsx — a repo-wide check confirms those are the only ones.
  • No scope creep; every change serves the stated goal.

What the suite does not exercise: the end-to-end chain with a live request. Each hop is unit-tested (this PR pins argv/settings → Config; #9565's suite pins Config → prompt assembly), but nobody has run them together against a model — the author says so plainly ("not run here — unit tests only"), and this review is an unattended CI run, so PR code is never executed here either. That sentence is the gap; the next section names the remedy.

Test evidence

Per the unattended-CI rule I did not build or run any PR code; the evidence is the PR's own CI on the reviewed commit, fetched via the API. Test (ubuntu-latest, Node 22.x) — the unit suite that carries this change — was still running at review time; the Qwen Triage Finalize job rewrites the table below once CI settles. macOS/Windows unit lanes and the CLI integration lane are skipped by design on pull requests (ci.yml runs them only for merge queue / scheduled / manual triggers), so those skips are expected, not a gap in this PR.

Final CI results for 0b34f32 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Test (ubuntu-latest, Node 22.x) ❌ failure
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
route ✅ success
Secret scan (TruffleHog) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle the remaining claim: @qwen-code /verify — that --output-style Concise (or general.outputStyle) actually changes the system prompt sent to the model, A/B against the base build with a wire oracle. That end-to-end effect is the one thing neither the diff nor the unit suite demonstrates, since each hop is tested in isolation but the chain is never executed.

Not verified: live-model behavior of the styles (no e2e run exists in this PR or in this review); the /settings surface (intentionally deferred — showInDialog: false, picker lands in the next slice).

中文说明

代码审查

对照下述 commit 的 base 审阅了完整 diff(6 个文件,+145 / -0)。无阻塞性问题,无约定违规。

  • 实现复用 feat(core): add the output-style layer to the system prompt #9565 的接口而非重复造轮子:getBuiltInOutputStyle / BUILT_IN_OUTPUT_STYLES 来自 core,解析出的定义直接进入既有的 Config outputStyle 参数。唯一的新逻辑是 resolveOutputStyle(优先级、default、未知名称警告),它属于 CLI 层,并与 loadCliConfig 中已有的 TLS 警告模式一致(debugLogger.warn + console.error('WARNING: …'))。
  • 整条链路已静态追踪:loadCliConfigConfig.outputStylegetMainSessionBaseSystemPrompt(client.ts)→ getCoreSystemPromptapplyOutputStyleArenaManager 也会取用该风格。这不是一个死开关。
  • 文档描述与代码相符:Learning 在 headless 运行中被跳过,QWEN_SYSTEM_MD 覆盖时忽略风格,未知名称的警告会指明来源(--output-style 还是 general.outputStyle)。
  • 测试覆盖了关键场景:默认未设置、大小写不敏感解析、参数优先于设置项、default 作为关闭手段、以及两个入口各自的未知名称警告且来源归属正确。新增的必填字段 CliArgs.outputStyle 已在 gemini.test.tsx 全部四处手写构造字面量中更新 —— 全仓库检查确认仅此四处。
  • 无范围蔓延;每处改动都服务于既定目标。

套件没有覆盖的部分:用真实请求跑端到端链路。每一跳都有单元测试(本 PR 钉住 argv/设置 → Config#9565 的套件钉住 Config → 提示词组装),但没有人把它们连起来对着真实模型跑过 —— 作者明确说明("本 PR 未实际运行,仅单元测试"),而本次评审是无人值守的 CI 运行,同样不会执行 PR 代码。这句话就是缺口所在;下一节给出了补救手段。

测试证据

按无人值守 CI 规则,未构建或运行任何 PR 代码;证据来自 PR 自身在受审 commit 上的 CI,通过 API 获取。承载本改动的单元测试 Test (ubuntu-latest, Node 22.x) 在评审时仍在运行;Qwen Triage Finalize 任务会在 CI 结束后更新下表。macOS/Windows 单元测试与 CLI 集成测试在 pull request 上按设计跳过(ci.yml 仅在合并队列/定时/手动触发时运行),这些跳过是预期行为,不是本 PR 的缺口。

沙盒验证可以补齐剩余论断:@qwen-code /verify —— --output-style Concise(或 general.outputStyle)是否真的改变了发给模型的系统提示词,A/B 对比 base 构建、以 wire oracle 为判据。这一端到端效果是 diff 和单元测试都未能证明的唯一事项,因为每一跳都是分开测试的,整条链从未被执行。

未验证:风格的真实模型效果(本 PR 与本次评审均无端到端运行);/settings 界面(有意延后 —— showInDialog: false,选择器在下一个切片落地)。

Qwen Code · qwen3.8-max

Reviewed at 0b34f321cf601e0c1a132a22c43ea276ba69dff3 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean static review and every claim in the PR body checked out against the base code; the two reservations are that the unit suite was still running on this commit at review time, and the end-to-end prompt-on-the-wire effect rests on the /verify lane named above.

This is a well-cut slice. My independent read of the problem — "the #9565 styles need a selector; resolve it in loadCliConfig like every other CLI-facing option; warn instead of failing on unknown names" — is exactly what landed, and I didn't find a simpler path the PR missed. Every base-code assertion in the description verified: Learning really is skipped in headless runs, QWEN_SYSTEM_MD really ignores the style, the unknown-name warning mirrors the TLS precedent in the same file, and the four updated test literals are the only hand-built CliArgs in the repo. The slice discipline is the part I'd thank the author for in six months: the /settings picker, the per-turn reminder, and custom style files are all explicitly deferred to follow-ups instead of being smuggled in.

Nothing to request changes for; no blocking findings. Approving — but the unit suite (Test (ubuntu-latest, Node 22.x)) was still running at review time, so approval is deferred until CI lands green on this commit.

中文说明

Confidence: 4/5 —— 静态审查干净,PR 正文中的每一项论断都已对照 base 代码核实;两点保留意见:评审时该 commit 的单元测试套件仍在运行,且"提示词真正上送"的端到端效果依赖上文指出的 /verify 通道。

这是一个切分得当的切片。我对问题的独立判断 —— "#9565 的风格需要一个选择器;像其他所有面向 CLI 的选项一样在 loadCliConfig 中解析;未知名称警告而不是失败" —— 与本 PR 的实现完全一致,我也没有找到被它遗漏的更简路径。描述中关于 base 代码的每一项论断都已验证:Learning 在 headless 运行中确实被跳过,QWEN_SYSTEM_MD 确实忽略风格,未知名称警告与同文件中 TLS 的先例一致,更新的四处测试字面量是全仓库仅有的手写 CliArgs 构造。这种切片纪律正是六个月后会让人感谢作者的地方:/settings 选择器、每轮提醒和自定义风格文件都被明确推迟到后续切片,而不是夹带进来。

没有需要请求修改的事项,也没有阻塞性问题。批准 —— 但评审时单元测试套件(Test (ubuntu-latest, Node 22.x))仍在运行,因此批准推迟到 CI 在该 commit 上变绿之后。

Qwen Code · qwen3.8-max

Reviewed at 0b34f321cf601e0c1a132a22c43ea276ba69dff3 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Test Plan (not a blocker): src/config/config.test.tsno such file or directory.

中文说明

Test Plan(非阻断):src/config/config.test.tsno such file or directory

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/cli/src/config/settingsSchema.ts Outdated
Comment thread packages/cli/src/config/config.test.ts
Comment thread docs/users/features/headless.md
Comment thread docs/users/configuration/settings.md
Comment thread docs/users/features/headless.md
…ngs schema

Review findings on QwenLM#10283:

- R1-1: `resolveOutputStyle` called `.trim()` on an unvalidated value, so a
  hand-edited `"general": { "outputStyle": 1 }` — or a repeated
  `--output-style`, which yargs delivers as an array despite
  `type: 'string'` — threw a TypeError inside `loadCliConfig` and killed
  every launch. Both inputs are now `unknown` and a non-string value warns
  and falls back to the default style, the same no-lockout path an unknown
  name already took. Picking the source now also survives a null argv.
- R1-2: regenerate `settings.schema.json`, which the new `SETTINGS_SCHEMA`
  entry never reached. This is what reddened "Check settings schema is
  up-to-date" in the Test job.
- R1-3: `general.outputStyle` is read once at startup and frozen into
  `Config` — nothing applies a mid-session change — so `requiresRestart` is
  now `true`, matching `general.outputLanguage`.
- R1-4: the `default` sentinel and the empty-value early return are now
  pinned by tests that also assert no warning was printed.
- R1-5/R1-6/R1-7: add `--output-style` to both reference tables that list
  its `--system-prompt` siblings, and document that a style is dropped when
  `--system-prompt` or `QWEN_SYSTEM_MD` replaces the built-in prompt.

Mutation-verified: dropping the typeof guard reddens 5 tests with
`raw.trim is not a function`; dropping the `default` sentinel reddens 2;
dropping `.trim()` reddens 1; flipping `requiresRestart` back reddens the
schema test.

Claude-Session: https://claude.ai/code/session_01VXsC4f71S6U6YkW82NRw7m

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x). Reviewed.

Test Plan (not a blocker): 418 tests pass — this review observed 25176, 605 passed.

中文说明

⚠️ 已从批准降级为评论:CI failing: Test (ubuntu-latest, Node 22.x)。 已审查。

Test Plan(非阻断):418 tests pass — this review observed 25176, 605 passed

— qwen3.8-max via Qwen Code /review (v0.22.2)

@qwen-code-dev-bot

qwen-code-dev-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 1 finishedview run. See this round's report below.

中文说明

AutoFix 第 1 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix round 3 — no action needed

No code changes were required this round. All seven round-1 findings were re-verified by execution against the current head 72689c3 and confirmed already fixed in the code, and the round-2 "CI failing: Test (ubuntu-latest, Node 22.x)" observation was diagnosed: the check is now SUCCESS on the PR head.

Finding verification (all fixed in 72689c3, re-verified this round)

  • R1-1 (Critical) — non-string output style crashed startup: the typeof guard is in place. Mutation probe: deleting the guard reddens exactly the 5 pinned tests with raw.trim is not a function (four non-string setting values + the repeated --output-style flag case).
  • R1-2 (Critical) — stale settings schema artifact: re-running npm run generate:settings-schema produces zero diff against the committed settings.schema.json; the CI step Check settings schema is up-to-date reproduced locally passes. This step was the cause of the round-1 red Test check.
  • R1-3 (Suggestion)requiresRestart flag: now true (value is read once in loadCliConfig and frozen into Config). Mutation probe: flipping it back to false reddens the new settingsSchema.test.ts case.
  • R1-4 (Suggestion)default sentinel warning-blindness: both default tests (flag side and setting side) plus the new whitespace-only test assert console.error was never called. Mutation probes: deleting the default sentinel reddens both default tests; deleting .trim() reddens the whitespace test.
  • R1-5 (Suggestion)--output-style row added to the headless options table and the settings.md CLI-arguments table; both verified present and accurate against the code.
  • R1-6 (Suggestion) — the settings.md row documents the value list, the default sentinel, and the warn-and-fall-back behaviour; verified present.
  • R1-7 (Suggestion) — the replaced-prompt caveat bullet is present in headless.md; its claim was re-verified against getMainSessionBaseSystemPrompt (custom --system-prompt returns without a style) and getCoreSystemPrompt (QWEN_SYSTEM_MD skips applyOutputStyle).

CI diagnosis

The workflow's own checks snapshot for this round shows Test (ubuntu-latest, Node 22.x) = SUCCESS at the PR head. The round-2 review observed the failing state before the fix commit's checks had reported. Locally, every component of that CI job was reproduced:

  • Check settings schema is up-to-date step: passes (no diff after regeneration).
  • Full packages/cli unit suite (25,260 tests): green once this runner's own environment markers — which do not exist on CI — are removed. The local-only failures were all in files this PR does not touch and which import nothing the PR changed:
    • 5 sandbox session-id / image tests: this agent's shell exports SANDBOX=… (the runner itself is sandboxed) and the guards read that marker; they pass with env -u SANDBOX.
    • 12 settings.test.ts tests: this agent's shell exports QWEN_HOME=…, redirecting settings paths past the tests' os.homedir() mocks; they pass with env -u QWEN_HOME.
    • 2 cdCommand/directoryCommand tests: EACCES on mkdtemp directly under /home/github-runner, which is not writable in this container but is on CI.
    • 13 update / i18n / script-lint-isolation / conversation-runtime-ownership / server-default-bridge-wiring tests: all pass once the markers are removed.
    • 3 qwen serve live-host discovery tests: require a real port-bound listener, which this container restricts.

Nothing actionable remains; every thread is resolved in the code.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npm run generate:settings-schema + CI's porcelain freshness check — passed (zero diff)
  • vitest run src/config/config.test.ts src/config/settingsSchema.test.ts src/gemini.test.tsx (packages/cli, SANDBOX unset) — 494 passed
  • Full packages/cli vitest suite (25,260 tests) — 25,257 pass in a CI-equivalent env; the 3 remaining failures are the port-restricted live-host tests above, in code untouched by this PR
  • Mutation probes: delete typeof guard → 5 tests red; flip requiresRestart → 1 red; delete default sentinel → 2 red; delete .trim() → 1 red. All probes restored afterwards; git status clean, HEAD unchanged at 72689c3.
中文说明

Autofix 第 3 轮 —— 无需处理

本轮无需任何代码改动。第 1 轮的 7 条发现已全部针对当前 HEAD 72689c3 通过实际执行复核,确认均已在代码中修复;第 2 轮 "CI failing: Test (ubuntu-latest, Node 22.x)" 的观察也已诊断清楚:该检查在 PR HEAD 上现在为 SUCCESS

发现复核(均已在 72689c3 中修复,本轮重新验证)

  • R1-1(Critical)——非字符串输出风格导致启动崩溃:typeof 守卫已就位。变异探针:删除该守卫后,恰好 5 个钉住该行为的测试变红并报 raw.trim is not a function(4 个非字符串设置值 + 重复 --output-style 参数用例)。
  • R1-2(Critical)——settings schema 产物过期:重新运行 npm run generate:settings-schema 后与已提交的 settings.schema.json 零差异;CI 的 Check settings schema is up-to-date 步骤在本地复现通过。该步骤正是第 1 轮 Test 检查变红的原因。
  • R1-3(Suggestion)——requiresRestart 标志:现为 true(该值在 loadCliConfig 中只读取一次并固化进 Config)。变异探针:把标志改回 false 会让新增的 settingsSchema.test.ts 用例变红。
  • R1-4(Suggestion)——default 哨兵的告警盲区:两个 default 用例(参数侧与设置侧)加上新增的纯空白值用例均断言 console.error 从未被调用。变异探针:删除 default 哨兵会让两个 default 用例变红;删除 .trim() 会让空白值用例变红。
  • R1-5(Suggestion)——headless 参数表与 settings.md 的 CLI 参数表中均已补上 --output-style 行;已核对存在且与代码行为一致。
  • R1-6(Suggestion)——settings.md 的行涵盖了取值列表、default 哨兵与未知名称告警降级行为;已核对存在。
  • R1-7(Suggestion)——headless.md 中已补入"提示词被替换时风格不生效"的注意事项;其描述已对照代码复核:getMainSessionBaseSystemPrompt 在自定义 --system-prompt 时返回不含风格的提示词,getCoreSystemPromptQWEN_SYSTEM_MD 生效时跳过 applyOutputStyle

CI 诊断

本轮工作流自身的检查快照显示 Test (ubuntu-latest, Node 22.x) 在该 PR HEAD 上为 SUCCESS。第 2 轮评审观察到失败状态时,修复提交的检查尚未回报。本地已逐项复现该 CI 作业的组成部分:

  • Check settings schema is up-to-date 步骤:通过(重新生成后零差异)。
  • packages/cli 完整单元测试套件(25,260 个测试):在去掉本运行器自身的环境标记后全部为绿——这些标记在 CI 上并不存在。本地独有的失败全部位于本 PR 未触碰、也未导入本 PR 所改代码的文件中:
    • 5 个 sandbox session-id / 镜像测试:本 agent 的 shell 导出了 SANDBOX=…(运行器本身就在沙箱里),而这些守卫会读取该标记;env -u SANDBOX 后全部通过。
    • 12 个 settings.test.ts 测试:本 agent 的 shell 导出了 QWEN_HOME=…,使设置路径绕过了测试对 os.homedir() 的 mock;env -u QWEN_HOME 后全部通过。
    • 2 个 cdCommand/directoryCommand 测试:在 /home/github-runner 下直接 mkdtempEACCES——该目录在本容器中不可写,在 CI 上可写。
    • 13 个 update / i18n / script-lint-isolation / conversation-runtime-ownership / server-default-bridge-wiring 测试:去掉标记后全部通过。
    • 3 个 qwen serve live-host 发现测试:需要真实绑定端口的监听器,本容器对此有限制。

没有剩余可处理项;所有线索均已在代码中解决。

验证

  • npm run build —— 通过
  • npm run typecheck —— 通过
  • npm run lint —— 通过
  • npm run generate:settings-schema + CI 的 porcelain 新鲜度检查 —— 通过(零差异)
  • vitest run src/config/config.test.ts src/config/settingsSchema.test.ts src/gemini.test.tsx(packages/cli,SANDBOX 未设置)—— 494 通过
  • packages/cli 完整 vitest 套件(25,260 个测试)—— 在 CI 等价环境下 25,257 通过;剩余 3 个失败即上述受端口限制的 live-host 测试,位于本 PR 未触碰的代码中
  • 变异探针:删除 typeof 守卫 → 5 个测试变红;改回 requiresRestart → 1 个变红;删除 default 哨兵 → 2 个变红;删除 .trim() → 1 个变红。所有探针随后均已还原;git status 干净,HEAD 仍为 72689c3 未变。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Test Plan (not a blocker): 418 tests pass — this review observed 25528, 605 passed.

Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/config/config.ts:1528 — [probe] No test pins the raw === null early-return branch of resolveOutputStyle
  • packages/cli/src/config/config.ts:1556 — [probe] Style warning prints once per loadCliConfig call (sandbox host+child / ACP re-run it); TLS precedent self-dedupes
中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):418 tests pass — this review observed 25528, 605 passed

收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment on lines +1546 to +1549
const known = BUILT_IN_OUTPUT_STYLES.map((s) => s.name).join(', ');
warnAboutOutputStyle(
`Unknown output style "${name}" (from ${source}); using the default style. Available styles: ${known}.`,
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] R3-1: The unknown-style warning interpolates the raw, unsanitized general.outputStyle value into terminal output — a repo-committed workspace .qwen/settings.json can thereby inject ANSI/OSC control sequences into the terminal of any user who trusts the folder and starts the CLI, bypassing the codebase's own centralized sanitizer (stripAnsiAndControl, whose doc says it is for "ANY untrusted string that may reach a terminal — … values interpolated into error messages").

An attacker commits {"general": {"outputStyle": "<ESC>[2J<ESC>[H✅ Trusted. No issues found.<ESC>]0;pwned<BEL>"}}; a victim clones the repo and trusts the folder (required for any work there — mergeSettings merges workspace settings on trust, and general.outputStyle is not among the WORKSPACE_RESTRICTED_SETTINGS stripped from that scope), then runs qwen. loadCliConfig hits the unknown-name path and console.error emits the raw escape bytes to stderr at startup — before the TUI owns the screen, and directly into the captured stderr stream on qwen -p headless/CI runs. The repo author can thereby paint spoofed startup output over the real session start, hide this very warning, and deliver OSC sequences (window title and worse on terminals that honor them). The same raw string also goes to debugLogger.warn. The non-string invalid-value warning does not echo the value, so this interpolation is the only raw-value echo.

Witness (probe on the unmodified PR code at this commit, scratch tree): calling loadCliConfig with that setting value, the captured stderr reports emittedContainsESC: true, emittedContainsBEL: true, emittedContainsClearScreen: true — the raw clear-screen/cursor-home ESC sequences and the OSC title-set reach the terminal. Applying the fix below flips the probe to all-false, and the 16 pre-existing output-style tests still pass.

Fix (verified in the scratch tree): sanitize before the trim in the string path, so both the console and the debug log get the cleaned value:

const name = stripAnsiAndControl(raw).trim();

(import stripAnsiAndControl from @qwen-code/qwen-code-core — already exported, with ~52 existing call sites).

If you add this guard, please pin it with a test: load loadCliConfig with outputStyle: '\u001b[31mEVIL\u001b[0m' and assert the warning captured by the console.error spy contains no \u001b control character (and the style stays unset) — then confirm that test goes red when the sanitize call is removed; the existing 'warns about an unknown style…' test would not catch the regression since 'Verbose' carries no control characters.

中文说明

[Critical] R3-1:未知风格的警告把未经净化的原始 general.outputStyle 值直接插值进终端输出——仓库中提交的 workspace .qwen/settings.json 因此可以向任何信任该文件夹并启动 CLI 的用户的终端注入 ANSI/OSC 控制序列,绕过了代码库自己的集中净化器(stripAnsiAndControl,其文档明确说明它用于"任何可能到达终端的不可信字符串——……插入错误消息的值")。

攻击者提交 {"general": {"outputStyle": "<ESC>[2J<ESC>[H✅ Trusted. No issues found.<ESC>]0;pwned<BEL>"}};受害者克隆仓库并信任该文件夹(在那里做任何工作都必须信任——mergeSettings 在信任时合并 workspace 设置,而 general.outputStyle 不在从该作用域剥离的 WORKSPACE_RESTRICTED_SETTINGS 之列),然后运行 qwenloadCliConfig 走到未知名称分支,console.error 在启动时把原始转义字节打到 stderr——此时 TUI 尚未接管屏幕,而在 qwen -p headless/CI 运行中则直接进入被捕获的 stderr 流。仓库作者借此可以在真实会话启动之上覆盖伪造的启动输出、隐藏这条警告本身,并投递 OSC 序列(窗口标题,以及在支持的终端上更严重的行为)。同样的原始字符串也会进入 debugLogger.warn。非字符串的非法值警告不会回显该值,因此这处插值是唯一回显原始值的地方。

证据(在本提交的未修改代码上运行探针,临时树):以该设置值调用 loadCliConfig,捕获的 stderr 报告 emittedContainsESC: true, emittedContainsBEL: true, emittedContainsClearScreen: true——原始的清除屏幕/光标归位 ESC 序列与 OSC 标题设置确实到达了终端。应用下面的修复后探针翻转为全 false,且 16 个已有的 output-style 测试仍然通过。

修复(已在临时树中验证):在字符串分支的 trim 之前净化,使 console 与调试日志拿到的都是清理后的值:const name = stripAnsiAndControl(raw).trim();(从 @qwen-code/qwen-code-core 导入 stripAnsiAndControl——已导出,现有约 52 处调用)。

修复见证:如果加上这个守卫,请补一个测试钉住它——以 outputStyle: '\u001b[31mEVIL\u001b[0m' 加载 loadCliConfig,断言 console.error 监听器捕获到的警告中不含 \u001b 控制字符(且风格保持未设置)——然后移除净化调用,确认该测试变红;现有的 'warns about an unknown style…' 测试抓不到这个回归,因为 'Verbose' 不含任何控制字符。

— qwen3.8-max via Qwen Code /review (v0.22.2)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round summary — PR #10283

Commit: 6e4824e7ddfix(cli): sanitize unknown output style before echoing it (#10283)
Changed files: packages/cli/src/config/config.ts, packages/cli/src/config/config.test.ts
Base-conflict: none (--conflict false, no merge performed; git merge-tree --write-tree origin/main HEAD also reports a clean merge, read-only check).

Feedback points and dispositions

1. [rc:3877667701] Critical R3-1 — unknown-style warning echoes the raw general.outputStyle value → Resolved in code

Reproduced before implementing (as required): added the regression test first and ran it against the unmodified PR code — it FAILED with the warning still emitting the raw \u001b[31mEVIL\u001b[0m escape sequences (assertion for the sanitized "EVIL" name was false, i.e. the control bytes were intact in the emitted warning).

Fix (the exact minimal change suggested by the finding): in resolveOutputStyle, the string path now sanitizes before trimming:

const name = stripAnsiAndControl(raw).trim();

stripAnsiAndControl is imported from @qwen-code/qwen-code-core (already exported, ~52 existing call sites, including the CLI's own sandboxConfig.ts). Both the console.error warning and debugLogger.warn now receive only the sanitized name. A value consisting solely of escape sequences sanitizes to empty and is treated as "no style" silently, consistent with the existing whitespace-only behavior.

Witness test added: 'strips control sequences before echoing an unknown style name' — loads loadCliConfig with outputStyle: '\u001b[31mEVIL\u001b[0m', asserts the style stays unset, the warning names the sanitized "EVIL", and no emitted console.error message contains any control character (\u0000-\u001f, \u007f). The pre-existing 'Verbose' test cannot catch this regression since it carries no control characters, hence the new test.

Mutation probe (recorded): temporarily reverted the guard to raw.trim(), re-ran the new test → FAILED (red); restored the guard, re-ran → 14/14 output-style tests green. The guard is witnessed.

2. [rv:5047654096] CHANGES_REQUESTED review body ("Partially reviewed — gaps disclosed") → No action requested this round

  • The two items the reviewer explicitly deferred under the convergence posture (round 3) — the untested raw === null early-return branch, and the once-per-loadCliConfig warning dedup observation — remain deferred exactly as the reviewer recorded them ("recorded, not requested in this round"). Nothing was implemented for them this round.
  • The disclosed gap (Integration Tests (CLI, No Sandbox) skipped in CI and not run locally) is informational: that job only runs on merge_group events per ci.yml, so it was SKIPPED in this PR's check run, not failed.

3. Failed check: Test (ubuntu-latest, Node 22.x)Diagnosed from local evidence; one real cause deferred to follow-up

No GitHub credentials are available in this runner, so CI logs could not be fetched; the diagnosis below is from local surrogate reproduction. The check ran ~11 minutes before failing (checks.json), consistent with failing in the test-suite step rather than an early lint/schema step. Local evidence:

  • This sandbox shell leaks SANDBOX/QWEN_HOME env vars and has an unwritable /home/github-runner HOME. A full packages/cli suite run inside it showed 49 failures; re-running with those env vars unset (env -u SANDBOX -u QWEN_HOME ..., matching CI) and/or a writable HOME resolved every failure class EXCEPT one:
    • 18 gemini.test.tsx failures — all EACCES mkdir '/home/github-runner/.qwen'; pass 76/76 with HOME=/tmp/qwen-home-probe. Container artifact.
    • cdCommand/directoryCommandEACCES mkdtemp under the real HOME; pass 50/50 with a writable HOME. Container artifact.
    • config.test.ts (incl. all output-style tests), settings.test.ts, settingsSchema.test.ts, Footer.test.tsx, docs/extensions/ide command tests — all pass with the clean env.
    • 13 mermaidImageRenderer.test.ts tests still fail in the clean env. Root cause identified with a minimal probe: Node v22.23.2 has module-syntax detection enabled by default (only --no-experimental-detect-module exists to disable it), and it loads the test's extensionless fake mmdc/chafa scripts (#!/usr/bin/env node + require(...)) as ES modules → ReferenceError: require is not defined in ES module scope. The test file is byte-identical to origin/main (this PR does not touch it), so this failure pre-dates the PR and is outside its footprint → recorded in deferred-findings.json for follow-up (candidate fix there: write a {"type":"commonjs"} package.json next to the fake binaries in the temp bin dir).
  • Settings-schema freshness is not the cause: npm run generate:settings-schema produced no diff at the merge head.
  • The branch also merges cleanly into current origin/main (which advanced with the Gemini→Llm rename refactor(core,cli): rename generic Gemini identifiers to Llm #10124 after this branch's last main-merge): git merge-tree --write-tree is conflict-free, and the PR's changed files don't collide with the rename (gemini.test.tsx hunk only adds outputStyle: undefined to mock objects).

Conclusion: nothing in this PR's changes fails locally; the red check is best explained by the pre-existing Node-22.x fake-binary incompatibility (identical on main) — the workflow's independent CI remains the final gate.

Verification

Commands actually run this round (final state unless noted):

  • npm run build — passed (exit 0; also run once pre-edit to bootstrap workspace dist/)
  • npm run typecheck — passed (exit 0)
  • npm run lint — passed (exit 0) after adding // eslint-disable-next-line no-control-regex in the new test (same precedent as packages/core/src/utils/textUtils.ts); initially failed on no-control-regex
  • npx prettier --check packages/cli/src/config/config.ts packages/cli/src/config/config.test.ts — passed
  • npm run generate:settings-schema — no schema drift (no settings source changed this round)
  • Reproduction (pre-fix): vitest run src/config/config.test.ts -t "strips control sequences" — 1 FAILED (defect reproduced on unmodified code)
  • Post-fix: vitest run src/config/config.test.ts -t "output style" — 14 passed
  • Mutation probe: guard removed → new test FAILED; guard restored → 14 passed
  • vitest run src/config/config.test.ts (full file, clean env) — 371 passed
  • vitest run src/config/settingsSchema.test.ts src/gemini.test.tsx (clean env) — settingsSchema passed; gemini 76/76 passed with a writable HOME (fails only with the container's unwritable /home/github-runner)
  • Full packages/cli suite (npm run test, then targeted re-runs with clean env / writable HOME) — all suites pass except mermaidImageRenderer.test.ts (13 tests, pre-existing on main, deferred) and the two HOME-permission tests (container artifact, pass with writable HOME)
  • git merge-tree --write-tree origin/main HEAD — clean merge (read-only)

Focused vitest runs used env -u SANDBOX -u QWEN_HOME -u QWEN_CODE_CUSTOM_SANDBOX_IMAGE -u HOSTNAME because this agent shell runs inside a Qwen sandbox container that exports those variables; they do not exist in CI.

中文说明

Autofix 审查轮次总结 — PR #10283

提交:6e4824e7ddfix(cli): sanitize unknown output style before echoing it (#10283)
变更文件:packages/cli/src/config/config.tspackages/cli/src/config/config.test.ts
与 base 的冲突:无(--conflict false,未执行合并;git merge-tree --write-tree origin/main HEAD 同样报告可干净合并,为只读检查)。

反馈点及处理

1. [rc:3877667701] Critical R3-1 — 未知风格警告回显未经净化的原始 general.outputStyle 值 → 已在代码中解决

实现前先复现(按要求):先加入回归测试并在未修改的 PR 代码上运行——测试失败,警告仍在原样输出 \u001b[31mEVIL\u001b[0m 转义序列(对净化后 "EVIL" 名称的断言为 false,即控制字节完整出现在警告输出中)。

修复(与 finding 建议完全一致的最小改动):resolveOutputStyle 的字符串分支现在先净化再 trim:

const name = stripAnsiAndControl(raw).trim();

stripAnsiAndControl@qwen-code/qwen-code-core 导入(已导出,现有约 52 处调用,包括 CLI 自己的 sandboxConfig.ts)。console.error 警告与 debugLogger.warn 现在都只接收净化后的名称。仅由转义序列组成的值净化后为空串,会被静默视为"无风格",与现有的纯空白值行为一致。

新增见证测试:'strips control sequences before echoing an unknown style name' —— 以 outputStyle: '\u001b[31mEVIL\u001b[0m' 加载 loadCliConfig,断言风格保持未设置、警告中显示净化后的 "EVIL",且 console.error 输出的任何消息都不含控制字符(\u0000-\u001f\u007f)。已有的 'Verbose' 测试无法捕获此回归,因为它不含任何控制字符,故新增该测试。

变异探针(记录在案): 临时将守卫还原为 raw.trim(),重跑新测试 → 失败(红);恢复守卫后重跑 → 14/14 output-style 测试全绿。该守卫有真实覆盖见证。

2. [rv:5047654096] CHANGES_REQUESTED 审查正文("部分审查——已披露缺口")→ 本轮无要求处理的事项

  • 审查者在收敛姿态(第 3 轮)下明确延后的两项——未加测试的 raw === null 提前返回分支,以及"警告每次 loadCliConfig 调用都打印一次"的去重观察——按审查者的记录保持延后("已记录,本轮不要求修改")。本轮未为其实现任何内容。
  • 披露的缺口(Integration Tests (CLI, No Sandbox) 在 CI 中被跳过且本地未运行)为信息性说明:按 ci.yml,该 job 仅在 merge_group 事件运行,因此在本 PR 的检查中为 SKIPPED,并非失败。

3. 失败检查:Test (ubuntu-latest, Node 22.x)已基于本地证据诊断;一个真实原因已延后至后续处理

本 runner 无 GitHub 凭据,无法拉取 CI 日志;以下诊断来自本地替代复现。该检查运行约 11 分钟后失败(checks.json),与在测试套件步骤失败(而非靠前的 lint/schema 步骤)相符。本地证据:

  • 本沙箱 shell 泄漏 SANDBOX/QWEN_HOME 环境变量,且 /home/github-runner HOME 不可写。在其中跑完整 packages/cli 套件出现 49 个失败;取消这些环境变量(env -u SANDBOX -u QWEN_HOME ...,与 CI 一致)并/或使用可写 HOME 重跑后,除一类外全部解决:
    • 18 个 gemini.test.tsx 失败——全部为 EACCES mkdir '/home/github-runner/.qwen';使用 HOME=/tmp/qwen-home-probe 后 76/76 通过。容器环境问题。
    • cdCommand/directoryCommand——在真实 HOME 下 EACCES mkdtemp;可写 HOME 下 50/50 通过。容器环境问题。
    • config.test.ts(含全部 output-style 测试)、settings.test.tssettingsSchema.test.tsFooter.test.tsx、docs/extensions/ide 命令测试——干净环境下全部通过。
    • 13 个 mermaidImageRenderer.test.ts 测试在干净环境下仍然失败。 已用最小探针定位根因:Node v22.23.2 默认启用模块语法检测(只有 --no-experimental-detect-module 可关闭),它把测试中无扩展名的假 mmdc/chafa 脚本(#!/usr/bin/env node + require(...))按 ES 模块加载 → ReferenceError: require is not defined in ES module scope。该测试文件与 origin/main 逐字节相同(本 PR 未触碰),因此该失败早于本 PR 且在其足迹之外 → 已记入 deferred-findings.json 待后续处理(候选修复:在临时 bin 目录中假二进制旁写一个 {"type":"commonjs"}package.json)。
  • 设置 schema 新鲜度不是原因:npm run generate:settings-schema 在合并头上无任何 diff。
  • 分支也能干净合入当前 origin/main(main 在本分支上次合并后又前进了 Gemini→Llm 重命名 refactor(core,cli): rename generic Gemini identifiers to Llm #10124):git merge-tree --write-tree 无冲突,且本 PR 的变更文件与重命名不冲突(gemini.test.tsx 的改动只是向 mock 对象添加 outputStyle: undefined)。

结论:本 PR 的变更在本地无任何失败;红色检查最合理的解释是主分支上已存在的 Node 22.x 假二进制不兼容(main 上同样失败)——以工作流的独立 CI 为最终验证关口。

验证

本轮实际执行的命令(除特别说明外均为最终状态):

  • npm run build — 通过(exit 0;编辑前还运行过一次以引导构建 workspace dist/)
  • npm run typecheck — 通过(exit 0)
  • npm run lint — 通过(exit 0),在新测试中添加 // eslint-disable-next-line no-control-regex 之后(与 packages/core/src/utils/textUtils.ts 的先例一致);首次运行曾因 no-control-regex 失败
  • npx prettier --check packages/cli/src/config/config.ts packages/cli/src/config/config.test.ts — 通过
  • npm run generate:settings-schema — 无 schema 漂移(本轮未改动任何设置源文件)
  • 复现(修复前):vitest run src/config/config.test.ts -t "strips control sequences" — 1 个失败(在未修改代码上复现缺陷)
  • 修复后:vitest run src/config/config.test.ts -t "output style" — 14 个通过
  • 变异探针:移除守卫 → 新测试失败;恢复守卫 → 14 个通过
  • vitest run src/config/config.test.ts(整文件,干净环境)— 371 个通过
  • vitest run src/config/settingsSchema.test.ts src/gemini.test.tsx(干净环境)— settingsSchema 通过;gemini 在可写 HOME 下 76/76 通过(仅在容器不可写的 /home/github-runner 下失败)
  • 完整 packages/cli 套件(npm run test,随后以干净环境/可写 HOME 定点重跑)— 除 mermaidImageRenderer.test.ts(13 个测试,main 上已存在,已延后)与两个 HOME 权限测试(容器环境问题,可写 HOME 下通过)外全部通过
  • git merge-tree --write-tree origin/main HEAD — 干净合并(只读)

定点 vitest 运行使用了 env -u SANDBOX -u QWEN_HOME -u QWEN_CODE_CUSTOM_SANDBOX_IMAGE -u HOSTNAME,因为本 agent shell 运行在导出这些变量的 Qwen 沙箱容器内;CI 中不存在这些变量。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

⚠️ The branch received new commits while this round ran; they were merged into this push, but this round's verification predates that merge — re-check anything that landed mid-run. · 本轮运行期间分支收到了新的提交;本次推送已将其合并,但本轮验证在合并之前完成——请复查运行期间落地的改动。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Partially reviewed — gaps disclosed. Suggestions are inline.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • R4-3 No test pins the outputStyle null early-return branch — already reported as the round-3 deferral at config.ts:1528 (review 5047654096)

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): 418 tests pass — this review observed 25559, 605 passed.

Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/config/config.ts:1534 — [review] Repeated --output-style discards both names silently
  • docs/users/configuration/settings.md:83 — [review] outputStyle docs row omits the restart note
  • packages/cli/src/config/config.ts:1542 — [probe] Empty --output-style silently overrides the setting
  • packages/cli/src/config/config.ts:2226 — [probe] outputStyle setting survives --bare and --safe-mode

Convergence: round 4 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/cli/src/config/config.ts (findings in round 3; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):418 tests pass — this review observed 25559, 605 passed

收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。

收敛情况:第 4 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:packages/cli/src/config/config.ts(第 3 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment on lines +1539 to +1541
// A repo-committed .qwen/settings.json is untrusted input; strip control
// sequences so the warning cannot inject terminal escapes from it.
const name = stripAnsiAndControl(raw).trim();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R4-4: The sanitize-before-echo guard closes ANSI/OSC/C0-C1 injection exactly as its comment claims, but Unicode bidi and zero-width format characters (U+202A–U+202E, U+2066–U+2069, U+200B–U+200F, U+FEFF) pass through stripAnsiAndControl, so a repo-committed .qwen/settings.json can still visually reorder or spoof this warning line on bidi-aware terminals — the injection class the comment and the new test claim to close. A hostile repo can commit "general": { "outputStyle": "\u202E…reversed text…\u202C" } (or a zero-width suffix such as "Concise\u200b"); anyone who trusts the folder and runs qwen gets the warning rendered with live bidi overrides — masking that the style was rejected or presenting fabricated text — and Unknown output style "Concise\u200b" quotes what looks like the valid name. Impact is display-level only (a hostile checkout already grants stronger primitives), hence not Critical.

Witness (probe on the real built sanitizer at this commit): U+202A, U+202E, U+2066, U+2069, U+200B, U+200F, U+FEFF all survive ('A'+char+'B' keeps length 3 for each), while the control case '\u001b[31mEVIL\u001b[0m\u0007' strips to 'EVIL'; "Concise\u200b" survives strip + trim, fails getBuiltInOutputStyle, and emits the unknown-style warning quoting the look-alike name.

Fix centrally in packages/core/src/utils/textUtils.ts, per the function's own docstring — a bypass fixed there is fixed for every call site:

// eslint-disable-next-line no-control-regex
const CONTROL_CHARS_RE = /[\u0000-\u001f\u007f-\u009f\u200b-\u200f\u202a-\u202e\u2060-\u2069\u206a-\u206f\ufeff]/g;

If you add this, please pin it: extend the stripAnsiAndControl tests in packages/core/src/utils/textUtils.test.ts with a case asserting \u202E/\u2067/\u200B are stripped, and strengthen 'strips control sequences before echoing an unknown style name' in config.test.ts to reject zero-width/bidi characters too — removing the added ranges from the regex must turn both red.

中文说明

这处“先净化再回显”的守卫确实如其注释所声明的那样封闭了 ANSI/OSC/C0-C1 注入,但 Unicode 双向控制符与零宽格式字符(U+202A–U+202E、U+2066–U+2069、U+200B–U+200F、U+FEFF)会原样穿过 stripAnsiAndControl,因此仓库中提交的 .qwen/settings.json 仍然可以在支持双向文本的终端上对这条警告做视觉重排或伪造——这正是注释与新增测试声称要封闭的注入类别。攻击者可以提交 "general": { "outputStyle": "\u202E…反转文本…\u202C" }(或零宽后缀,如 "Concise\u200b");任何信任该文件夹并运行 qwen 的用户都会看到带着活跃双向控制符的警告——可以掩盖“风格被拒绝”这一事实,或呈现伪造的文本——并且 Unknown output style "Concise\u200b" 引用的是一个看起来合法的名字。影响仅限于显示层面(恶意检出本身已提供更强的攻击原语),因此不是 Critical。

证据(在本提交上对真实构建出的净化器运行探针):U+202A、U+202E、U+2066、U+2069、U+200B、U+200F、U+FEFF 全部存活(对每个字符 'A'+char+'B' 长度保持为 3),而对照 '\u001b[31mEVIL\u001b[0m\u0007' 被净化为 'EVIL'"Concise\u200b" 穿过净化与 trim 后无法匹配 getBuiltInOutputStyle,未知风格警告引用了这个看似合法的名字。

请按该函数自身文档字符串的约定在 packages/core/src/utils/textUtils.ts 中集中修复——在那里修复一处绕过,所有调用点同时得到修复(正则见上方英文代码块)。

如果加上此修复,请补测试钉住它:在 packages/core/src/utils/textUtils.test.tsstripAnsiAndControl 测试中新增断言 \u202E/\u2067/\u200B 被剥离的用例,并加强 config.test.ts 中的 'strips control sequences before echoing an unknown style name' 使其同样拒绝零宽/双向字符——从正则中移除新增范围后两个测试都应变红。

— qwen3.8-max via Qwen Code /review (v0.22.2)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Partially reviewed — gaps disclosed.

4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • repeated per-loadCliConfig style warning — already recorded as the round-3 deferral at config.ts:1556 (review 5047654096)
  • no test pins the outputStyle null early-return branch — already recorded as the round-3 deferral at config.ts:1528 (review 5047654096)
  • outputStyle setting survives --bare and --safe-mode — already recorded as the round-4 deferral at config.ts:2226 (review 5048388632)
  • outputStyle docs row omits the restart note — already recorded as the round-4 deferral at settings.md:83 (review 5048388632)

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): 418 tests pass — this review observed 23479, 605 passed.

Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/config/config.ts:1552 — [review] Unknown-style warning echoes the untrusted settings value with no length bound
  • packages/cli/src/config/config.ts:1534 — [review] Invalid-value warning names neither the received shape nor the duplicated flag
中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 4 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):418 tests pass — this review observed 23479, 605 passed

收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.2)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Partially reviewed — gaps disclosed.

3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • R6-1 outputStyle docs row omits the restart note — already recorded as the round-4 deferral at settings.md:83 (review 5048388632)
  • R6-3 No test pins the outputStyle null early-return branch — already recorded as the round-3 deferral at config.ts:1528 (review 5047654096)
  • R6-4 Empty --output-style silently overrides the setting — already recorded as the round-4 deferral at config.ts:1542 (review 5048388632)

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Test Plan (not a blocker): 418 tests pass — this review observed 25727, 605 passed.

Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/config/config.ts:1533 — [probe] Repeated --output-style with identical values is warned as invalid and the requested style is dropped
中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 3 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

Test Plan(非阻断):418 tests pass — this review observed 25727, 605 passed

收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants