Skip to content

feat(cli): add /output-style command with picker and mid-session switching - #10683

Merged
qqqys merged 3 commits into
QwenLM:mainfrom
qqqys:feat/output-style-command
Sep 2, 2026
Merged

feat(cli): add /output-style command with picker and mid-session switching#10683
qqqys merged 3 commits into
QwenLM:mainfrom
qqqys:feat/output-style-command

Conversation

@qqqys

@qqqys qqqys commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This adds /output-style so users can inspect, choose, and change the active output style from inside a session. Bare /output-style opens a picker with default and the four built-in styles, pre-selected on the effective style; /output-style <name> applies a style case-insensitively; and /output-style default clears it. In non-interactive and ACP sessions, the bare form reports the current style and options instead of opening a dialog.

A successful choice persists first: it writes a trusted workspace when that workspace owns general.outputStyle; otherwise it writes user settings. After that write succeeds, the command updates the live configuration and rebuilds the bound system instruction so the next turn uses the new style. Clearing persists the literal default in that owning scope. When a custom system prompt replaces the built-in prompt, or Learning is skipped in a headless run, the command saves the choice and reports the actual effective behavior. Bare and safe modes reject changes, and persistence failures leave the running style unchanged.

The generic settings path still marks general.outputStyle as requiring a restart because only /output-style performs the live refresh. The user documentation now describes the built-in styles and both configuration paths.

Why it's needed

#9565 shipped the styles and #10283 made them selectable at startup, but users still could not discover or change the effective style inside a running session. This closes that gap with the established /effort command shape while keeping the live refresh and persistence behavior explicit and testable.

Reviewer Test Plan

How to verify

Run npm run dev, then invoke /output-style. Confirm the picker opens with the effective style selected, choosing Concise reports success and changes the next response, and Escape or the dialog-close path cancels without mutation. Confirm /output-style concise applies case-insensitively, /output-style default clears and persists default, and an unknown name reports the valid choices without changing runtime or settings. In -p or ACP mode, confirm the bare command reports the current style and options instead of opening the picker.

For scope behavior, confirm a trusted workspace that owns the key is updated in workspace scope, while an untrusted or non-owning workspace writes user scope. Confirm bare and safe modes reject the command, a persistence error does not change the live style, and a live refresh failure still reports the persisted result.

Focused verification from packages/cli:

npx vitest run src/ui/commands/output-style-command.test.ts src/ui/hooks/use-output-style-command.test.ts src/ui/hooks/slashCommandProcessor.test.ts src/ui/hooks/useDialogClose.test.ts src/ui/components/OutputStyleDialog.test.tsx src/config/settingsSchema.test.ts

Result: 6 files passed, 206 tests passed. The ordering assertion was also mutation-checked: moving the live config update after system-instruction refresh makes both set and clear tests fail.

Evidence (Before & After)

Before: /output-style was not a command; the active style was invisible and changing it required editing settings and restarting.

After:

│ > Output Style (applies now and persists)                                    │
│                                                                              │
│ › 1. default — The standard prompt, with no extra style                      │
│   2. Concise — Answers first, with no preamble, narration, or closing recap… │
│   3. Proactive — Starts work immediately and prefers a stated assumption ove…│
│   4. Explanatory — Explains implementation choices and codebase patterns alo…│
│   5. Learning — Hands the user small, meaningful pieces of code to write, th…│
│                                                                              │
│ (Use Enter to select, Esc to cancel)                                         │
●︎ Output style set to Concise.
> hi, what does /effort do in one sentence?
◆︎ /effort sets how hard reasoning-capable models think (low, medium, high, xhigh, max — mapped and clamped per provider); run it bare to open the picker or pass a tier directly.
> /output-style Verbose
✕ Unknown output style "Verbose". Choose one of: Concise, Proactive, Explanatory, Learning, or "default" for no style.
> /output-style default
●︎ Output style cleared; responses use the default style.

Tested on

OS Status
🍏 macOS ✅ focused build and unit tests
🪟 Windows ⚠️ CI only
🐧 Linux ✅ interactive session

Environment (optional)

Node 22; npm run dev for the interactive session; focused Vitest coverage plus repository build for review fixes.

Risk & Scope

  • Main risk or tradeoff: a mid-session switch invalidates the cached stable prompt prefix once. If no chat is bound yet and the live rebuild fails, the choice remains persisted and the failure is logged.
  • Not validated / out of scope: custom .qwen/output-styles/*.md files, extension-provided styles, and a footer/status-line indicator are later slices.
  • Breaking changes / migration notes: none. The generic setting retains its restart hint; /output-style is the dedicated live-update path.

Linked Issues

Follows #9565, #10282, and #10283.

中文说明

这个 PR 做了什么

本 PR 新增 /output-style,让用户能在会话内查看、选择和更改当前输出风格。裸 /output-style 打开选择器,列出 default 和四个内置风格,并预选当前实际生效的风格;/output-style <name> 以大小写不敏感的方式直接应用;/output-style default 清除风格。在非交互和 ACP 会话中,裸命令报告当前风格和可选项,而不是打开对话框。

选择成功时先持久化:若可信工作区拥有 general.outputStyle,则写回工作区级设置;否则写入用户级设置。写入成功后,命令再更新当前运行配置并重建已绑定的系统指令,因此下一轮即可使用新风格。清除操作会在对应的拥有者作用域持久化字面量 default。当自定义系统提示词替换内置提示词,或 Learning 在 headless 模式下被跳过时,命令仍保存选择,并准确报告实际生效情况。Bare 和 safe 模式拒绝更改;持久化失败时不会改变运行中的风格。

通用设置路径仍将 general.outputStyle 标记为需要重启,因为只有 /output-style 会执行实时刷新。用户文档现已说明内置风格和两种配置路径。

为什么需要它

#9565 提供了输出风格,#10283 让启动时可以选择风格,但用户仍无法在运行中的会话里发现或更改实际风格。本 PR 使用既有的 /effort 命令形态补齐该能力,并让实时刷新和持久化行为都有明确测试覆盖。

评审者测试计划

如何验证

运行 npm run dev 后调用 /output-style。确认选择器打开并预选实际生效的风格;选择 Concise 后报告成功,下一轮回答采用新风格;Escape 或关闭对话框不会产生变更。确认 /output-style concise 大小写不敏感,/output-style default 清除并持久化 default,未知名称会列出有效选项且不改变运行时或设置。在 -p 或 ACP 模式下,裸命令应报告当前风格和可选项,而不是打开选择器。

作用域方面,确认拥有该键的可信工作区写回工作区级设置,不可信或不拥有该键的工作区写入用户级设置。确认 bare/safe 模式拒绝命令,持久化错误不会改变实时风格,实时刷新失败时仍报告已持久化的结果。

packages/cli 下执行上方 focused Vitest 命令:6 个文件、206 项测试全部通过。调用顺序还做了反向变异验证:将运行配置更新故意移动到系统指令刷新之后,两条顺序测试都会失败。

证据(Before & After)

Before:/output-style 不存在;当前风格不可见,更改风格需要编辑设置并重启。

After:见上方真实会话记录。会话中选择 Concise 后,紧接着的回答已经采用“结果先行、零铺垫”的风格;未知风格报错且不修改状态;default 恢复默认风格。

测试环境

操作系统 状态
🍏 macOS ✅ focused build 和单元测试
🪟 Windows ⚠️ 仅 CI
🐧 Linux ✅ 交互会话

风险与影响范围

  • 主要风险或权衡:会话中途切换会使缓存的稳定提示词前缀失效一次。若 chat 尚未绑定导致实时重建失败,选择仍会持久化,失败会记录到日志。
  • 未验证 / 不在范围内:自定义 .qwen/output-styles/*.md 文件、extension 提供的风格、footer/状态栏指示属于后续切片。
  • 破坏性改动 / 迁移说明:无。通用设置仍保留重启提示;/output-style 是专用的实时更新路径。

关联 Issue

承接 #9565#10282#10283

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

…ching

Bare /output-style opens a RadioButtonSelect picker (default plus the four
built-in styles); /output-style <name> applies directly, case-insensitively,
with 'default' selecting no style. A choice applies to the running session by
rebuilding the system instruction in place and persists to user settings.
general.outputStyle no longer requires a restart.

Follows QwenLM#10283 as the picker/live-switch slice it deferred.
@qqqys
qqqys force-pushed the feat/output-style-command branch from 02e32b6 to b839042 Compare September 1, 2026 07:04
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: this isn't a bug fix but the next slice in a planned series — #10283's description explicitly promised "the /output-style command that applies a change mid-session come in the next slice" (verified against that PR). The gap is real and user-visible: since #10283, styles can only be configured blind by editing settings.json and restarting, with no way to see or change the active style in-session. The PR carries real before/after session evidence showing the mid-session hand-off working.

Direction: aligned. This is the fourth step of a merged series (#9565, #10282, #10283, all by the same author, who has write access), and the live refreshSystemInstruction() hand-off is exactly the piece #10283 carved out for separate review. Claude Code ships the same area — its changelog has a built-in "Concise" output style selectable from its config UI plus several follow-up fixes — so the direction is well established.

Size: two core paths touched, both trivial — packages/cli/src/config/settingsSchema.ts (11 lines: the requiresRestart flip and a description update) and packages/cli/src/services/BuiltinCommandLoader.ts (2 lines: command registration). Overall ~413 production lines, 325 test lines, ~119 docs lines, 2 schema lines — under the 500-line awareness threshold.

Approach: the shape feels right. It follows the established /effort pattern (bare opens a picker, an argument applies directly) instead of forcing a soon-to-grow dynamic list into the static /settings schema, reuses the core APIs the earlier slices shipped (setOutputStyle + refreshSystemInstruction, the same mechanism /language uses), and shares one apply path between the dialog and the argument form. The two non-obvious decisions — persisting the literal default instead of deleting the key, and reporting "no effect" when the system prompt is replaced — are both justified in the design doc. Nothing I'd cut.

Risk: no elevated risk signals — none of the changed files match the revert-correlated paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:这不是 bug 修复,而是计划系列中的下一个切片 —— #10283 的描述明确承诺了"能在会话中途切换的 /output-style 命令会在下一个切片中到来"(已对照该 PR 核实)。缺口真实且用户可见:自 #10283 以来,风格只能通过编辑 settings.json 再重启来"盲配",会话内无法查看或更改当前风格。PR 附有真实会话的 before/after 证据,展示了会话中途切换的生效过程。

方向:对齐。这是已合并系列(#9565#10282#10283,同一作者,具有 write 权限)的第四步,实时的 refreshSystemInstruction() 交接正是 #10283 特意拆出来单独评审的部分。Claude Code 也在同一领域有对应功能 —— 其 changelog 有内置的 "Concise" 输出风格(在配置界面中选择)及多项后续修复 —— 方向成熟。

规模:触及两个核心路径,均极小 —— packages/cli/src/config/settingsSchema.ts(11 行:requiresRestart 翻转与描述更新)和 packages/cli/src/services/BuiltinCommandLoader.ts(2 行:命令注册)。整体约 413 行生产代码、325 行测试、约 119 行文档、2 行 schema —— 低于 500 行关注阈值。

方案:形态合理。沿用既有的 /effort 模式(裸命令开选择器、带参直接应用),而不是把即将增长的动态列表塞进静态的 /settings schema;复用前置切片提供的核心 API(setOutputStyle + refreshSystemInstruction,与 /language 同一机制);对话框与带参形式共享同一条应用路径。两个不直观的决策 —— 清除时持久化字面量 default 而非删键、系统提示词被替换时明确报告"不生效" —— 在设计文档中均有论证。没有可砍的部分。

风险:无升级风险信号 —— 改动文件均未命中与 revert 相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

My independent take before reading the diff: this wants a /effort-shaped command (bare opens a picker, argument applies directly) that reuses the style machinery the earlier slices shipped — setOutputStyle plus the refreshSystemInstruction() hand-off /language already uses — and persists like /language does. The PR lands exactly there, and I verified every integration point against the base tree rather than taking the description on faith:

  • BUILT_IN_OUTPUT_STYLES / getBuiltInOutputStyle are exported from core (case-insensitive lookup confirmed), and Config.setOutputStyle's docstring literally mandates the refreshSystemInstruction() follow-up this PR performs.
  • getLlmClient() is non-nullable, so the un-guarded call matches languageCommand.
  • resolveMainSessionOutputStyle returns nothing when --system-prompt / QWEN_SYSTEM_MD is active — the "saved but has no effect" note fires on the right condition.
  • RadioButtonSelect has no Escape handling of its own, so the dialog's useKeypress cancel can't double-fire; initialIndex pre-selection falls back to default (index 0) truthfully when nothing is configured.
  • The useDialogClose insertion, AppContainer wiring, UIState/UIActions additions, and the recordSlashCommand call all mirror the /effort precedent one-for-one.

No critical blockers found. One observation below the bar, for the author: the design doc justifies persisting the literal default on clear with "a workspace-scope setting cannot silently resurface on the next start" — but general.outputStyle is in neither WORKSPACE_RESTRICTED_SETTINGS nor WORKSPACE_NON_OVERRIDING_SETTINGS, and the merge is SystemDefaults → User → Workspace → System, so a trusted workspace value overrides the user scope at startup whether the user key holds default or is absent. The behavior shipped is fine either way; only that rationale sentence doesn't hold and might be worth rewording (or, if an explicit user choice should beat the workspace, that's a follow-up discussion, not this slice).

The live hand-off is the point of this PR, so here is the path a selection takes:

sequenceDiagram
    participant P1 as User
    participant P2 as output-style command and hook
    participant P3 as Config
    participant P4 as LlmClient
    participant P5 as LoadedSettings
    P1->>P2: pick a style in the dialog, or pass it as an argument
    P2->>P3: setOutputStyle(style, or undefined for default)
    P2->>P4: refreshSystemInstruction()
    P4-->>P2: system prompt rebuilt and rebound in place
    P2->>P5: setValue(User, general.outputStyle, name or default)
    P2-->>P1: report the outcome in chat
Loading
Files changed (23 of 23 shown)
File What changed
docs/design/2026-09-01-output-style-picker.md New design doc recording the shape decisions
docs/users/configuration/settings.md outputStyle row updated for mid-session switching, table reflowed by the longer cell
docs/users/features/_meta.ts Registers the new Output Styles page
docs/users/features/commands.md Command table gains the /output-style rows
docs/users/features/output-styles.md New user-facing Output Styles page
packages/cli/src/config/settingsSchema.ts requiresRestart flips to false, description mentions /output-style
packages/cli/src/config/settingsSchema.test.ts Pins the requiresRestart flip
packages/cli/src/services/BuiltinCommandLoader.ts Registers the command
packages/cli/src/ui/AppContainer.tsx Wires the hook into state, actions, and dialog gating, mirroring effort
packages/cli/src/ui/commands/output-style-command.ts The slash command: bare opens the picker or lists, argument applies, unknown errors
packages/cli/src/ui/commands/output-style-command.test.ts 8 tests pinning every command branch
packages/cli/src/ui/commands/output-style-utils.ts Shared apply path: set, refresh, persist, build the feedback message
packages/cli/src/ui/commands/types.ts Adds output-style to the dialog union
packages/cli/src/ui/components/DialogManager.tsx Renders OutputStyleDialog when open
packages/cli/src/ui/components/OutputStyleDialog.tsx RadioButtonSelect picker with default pre-selection and Esc cancel
packages/cli/src/ui/components/OutputStyleDialog.test.tsx Render and Escape-handling tests
packages/cli/src/ui/contexts/UIActionsContext.tsx handleOutputStyleSelect in the actions interface
packages/cli/src/ui/contexts/UIStateContext.tsx isOutputStyleDialogOpen in the state interface
packages/cli/src/ui/hooks/slashCommandProcessor.ts Dialog-open case and bare-command invocation hiding
packages/cli/src/ui/hooks/use-output-style-command.ts Dialog state plus the async apply-and-report handler
packages/cli/src/ui/hooks/use-output-style-command.test.ts Hook tests: open, apply, clear, cancel-without-mutation
packages/cli/src/ui/hooks/useDialogClose.ts Closing via the dialog-close path mimics Esc
packages/vscode-ide-companion/schemas/settings.schema.json Description sync

Testing

This is an unattended CI run: triage read the PR's own CI checks through the API and executed none of the PR's code. The tmux transcript in the PR body is the author's claim from a real Linux session, not independently re-run here. Note for the record: the commit originally pushed (02e32b6…) was amended/force-pushed to the head reviewed here, cancelling its CI run; the new diff is byte-identical to the one reviewed, so this pass carries over. Unit/integration suites for the reviewed head are still running at the time of writing — the table below reflects that snapshot, and the finalize job updates it once CI settles.

Check Conclusion
Test (ubuntu-latest, Node 22.x) ⏳ in progress
Integration Tests (no-AK, No Sandbox) ⏳ in progress
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
precheck-pr / precheck ✅ success
Dependency CVE audit ✅ success
Secret scan (TruffleHog) ✅ success
Classify PR / assign / label / authorize ✅ success
Test (macos-latest / windows-latest, Node 22.x) ⏭️ skipped

What the unit tests do pin (from the diff): every command branch, the hook's apply/clear/cancel paths, the requiresRestart flip, and the dialog's render/cancel. What they cannot pin is the behavioural heart of the PR — that the picker actually opens with the current style pre-selected, that the very next turn answers in the new style, and that non-interactive mode lists instead of opening a dialog — because those live in the rendered TUI and the model round-trip, which is exactly what the author's single Linux session demonstrates only as a claim.

Sandboxed verification would settle this: @qwen-code /tmux — that the picker opens pre-selected, a selection applies mid-session and the next turn carries the style, /output-style Verbose errors without touching settings, and -p mode lists instead of opening a dialog; and @qwen-code /verify — A/B proof against the base build that the command wiring actually changes session behaviour rather than only passing its own tests. The author has write access, so a maintainer can trigger either directly.

中文说明

代码审查

在读 diff 之前我的独立方案是:做一个 /effort 形态的命令(裸命令开选择器、带参直接应用),复用前置切片提供的风格机制 —— setOutputStyle 加上 /language 已在使用的 refreshSystemInstruction() 交接 —— 并像 /language 一样持久化。PR 的实现与此完全一致,且我逐一对照 base 代码核实了每个集成点,而不是轻信描述:

  • BUILT_IN_OUTPUT_STYLES / getBuiltInOutputStyle 确由 core 导出(大小写不敏感查找已确认),Config.setOutputStyle 的 docstring 本身就要求调用方跟进 refreshSystemInstruction() —— 本 PR 正是这么做的。
  • getLlmClient() 返回非空类型,不带 ?. 的调用与 languageCommand 一致。
  • --system-prompt / QWEN_SYSTEM_MD 生效时 resolveMainSessionOutputStyle 返回空 —— "已保存但本会话不生效"的提示在正确的条件下触发。
  • RadioButtonSelect 自身不处理 Escape,对话框 useKeypress 的取消不会双重触发;未配置风格时 initialIndex 诚实地回落为 default(第 0 项)。
  • useDialogClose 插入、AppContainer 接线、UIState/UIActions 新增、recordSlashCommand 调用,均与 /effort 先例一一对应。

未发现致命阻塞问题。一条低于门槛的观察,供作者参考:设计文档用"工作区级设置不会在下次启动时悄悄卷土重来"来论证清除时持久化字面量 default —— 但 general.outputStyle 既不在 WORKSPACE_RESTRICTED_SETTINGS 也不在 WORKSPACE_NON_OVERRIDING_SETTINGS,合并顺序为 SystemDefaults → User → Workspace → System,因此受信工作区的值在启动时总会覆盖用户级设置,无论用户键是 default 还是不存在。两种写法下实际行为都没问题;只是那句论证站不住,建议改写(若认为用户的显式选择应当压过工作区,那是后续讨论,不属于本切片)。

中途切换是本 PR 的核心,选择路径如下(图:用户 → 命令/钩子:选择风格 → Config:setOutputStyle → LlmClient:refreshSystemInstruction,原地重建系统提示 → LoadedSettings:持久化到用户级 → 回到用户:在对话中反馈结果)。

测试

这是无人值守的 CI 运行:triage 通过 API 读取 PR 自身的 CI 检查,未执行任何 PR 代码。PR 正文中的 tmux 录屏是作者在真实 Linux 会话中的自述,未在此独立复跑。记录在案:最初推送的提交(02e32b6…)被 amend/force-push 为当前评审的 head,旧 CI 运行被取消;新 diff 与已审版本逐字节一致,因此本次审查结论延续有效。评审时评审头的单元/集成测试仍在运行 —— 下表为该时刻快照,finalize 任务会在 CI 结束后更新。

单测钉住的内容(依据 diff):命令的所有分支、钩子的应用/清除/取消路径、requiresRestart 翻转、对话框渲染与取消。单测钉不住的是行为核心 —— 选择器确实以当前风格预选打开、下一轮回答确实带上新风格、非交互模式列出选项而非开对话框 —— 这些活在渲染出的 TUI 和模型往返里,正是作者单平台(Linux)会话自述所覆盖、而此处无法独立核实的部分。

沙箱验证可以定论:@qwen-code /tmux —— 选择器以当前风格预选打开、选中后会话中途生效且下一轮带上新风格、/output-style Verbose 报错且不触碰设置、-p 模式列出选项而不开对话框;以及 @qwen-code /verify —— 相对 base 构建的 A/B 证据,证明命令接线确实改变会话行为而不只是让自己的测试通过。作者具有 write 权限,维护者可直接触发任一指令。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean review; the code follows the repo's own precedent so faithfully that the only open items are one inaccurate design-doc sentence and CI that hasn't settled.

Stepping back: this is exactly the slice #10283 carved out for separate review, and it lands the shape that PR deferred — no more, no less. The implementation is the boring version of this feature in the best sense: one shared apply path for the dialog and argument forms, reuse of the existing style machinery and the /language refresh mechanism instead of a parallel one, persistence scoped like /language, and tests that pin every command branch. The one thing I'd flag for future maintainers is the design doc's rationale for persisting the literal default — it claims a protection against workspace-scope settings that the settings merge does not actually provide (workspace outranks user for this key), as detailed in the review comment. That's wording, not behavior, and it doesn't block.

What keeps this at 4 rather than 5 is evidence, not code: the unit suite pins the command logic deterministically, but what users will actually feel — the picker opening pre-selected on the current style, the very next turn answering in the new style — rests so far on the author's single-Linux-session transcript, and CI for the reviewed head is still running. Approving now would attest to a green suite that doesn't exist yet, so approval is deferred until CI lands green on b83904298268b35528785b15ded1f65fcdff4354; the finalize step posts the same commit-pinned approval then, and withholds it if anything lands red or the head moves again. A maintainer who wants live proof before merge can trigger the sandboxed lanes named in the review comment.

中文说明

置信度:4/5 —— 干净的审查;代码忠实沿用仓库自身先例,遗留项只有设计文档中一句不准确的表述,以及尚未结束的 CI。

退一步看:这正是 #10283 拆出来单独评审的切片,落地的也正是那个 PR 延后的形态 —— 不多不少。实现在最好的意义上是这个功能"无聊"的版本:对话框与带参形式共享一条应用路径,复用既有风格机制与 /language 的刷新机制而不是另起炉灶,持久化作用域与 /language 一致,测试钉住命令的每个分支。唯一要提醒未来维护者的是设计文档中持久化字面量 default 的论证 —— 它声称了对工作区级设置的防护,而设置合并对这一键实际上并不提供(工作区优先于用户),详见审查评论。那是措辞问题,不是行为问题,不构成阻塞。

停在 4 而不是 5 的原因是证据,不是代码:单测确定性地钉住了命令逻辑,但用户真正会感受到的部分 —— 选择器以当前风格预选打开、下一轮回答带上新风格 —— 目前仍只有作者单 Linux 会话的录屏为证,且评审头的 CI 仍在运行。现在批准等于为一个尚不存在的绿色结果背书,因此批准推迟到 b83904298268b35528785b15ded1f65fcdff4354 的 CI 变绿后执行;届时 finalize 步骤会发布同样钉在该提交上的批准,若有检查变红或 head 再次移动则不会执行。维护者如想在合并前拿到现场证据,可触发审查评论中提到的沙箱指令。

Qwen Code · qwen3.8-max

Reviewed at b83904298268b35528785b15ded1f65fcdff4354 · 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.

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): src/ui/commands/output-style-command.test.tsno such file or directory; src/ui/hooks/use-output-style-command.test.tsno such file or directory; src/ui/components/OutputStyleDialog.test.tsxno such file or directory; src/config/settingsSchema.test.tsno such file or directory.

中文说明

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

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

Test Plan(非阻断):src/ui/commands/output-style-command.test.tsno such file or directory; src/ui/hooks/use-output-style-command.test.tsno such file or directory; src/ui/components/OutputStyleDialog.test.tsxno such file or directory; src/config/settingsSchema.test.tsno such file or directory

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

Comment thread packages/cli/src/ui/commands/output-style-command.ts
Comment thread packages/cli/src/ui/commands/output-style-utils.ts Outdated
Comment thread packages/cli/src/ui/commands/output-style-utils.ts Outdated
Comment thread packages/cli/src/ui/commands/output-style-utils.ts Outdated
Comment thread packages/cli/src/ui/commands/output-style-command.ts Outdated
Comment thread docs/design/2026-09-01-output-style-picker.md Outdated
Comment thread packages/cli/src/ui/hooks/slashCommandProcessor.ts
Comment thread packages/cli/src/ui/components/OutputStyleDialog.tsx
Comment thread packages/cli/src/ui/hooks/use-output-style-command.ts
Comment thread packages/cli/src/ui/hooks/useDialogClose.ts

@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.

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): src/ui/commands/output-style-command.test.tsno such file or directory; src/ui/hooks/use-output-style-command.test.tsno such file or directory; src/ui/components/OutputStyleDialog.test.tsxno such file or directory; src/config/settingsSchema.test.tsno such file or directory.

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

  • packages/cli/src/ui/commands/output-style-command.ts:36 — [probe] supportedModes contract pinned by no test — deferred under the rounds-2-5 code-age rule (line unchanged since the previous round's reviewed head)
中文说明

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

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

Test Plan(非阻断):src/ui/commands/output-style-command.test.tsno such file or directory; src/ui/hooks/use-output-style-command.test.tsno such file or directory; src/ui/components/OutputStyleDialog.test.tsxno such file or directory; src/config/settingsSchema.test.tsno such file or directory

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

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

Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/cli/src/ui/commands/output-style-command.test.ts
Comment thread packages/cli/src/ui/commands/output-style-utils.ts
Comment thread packages/cli/src/ui/hooks/slashCommandProcessor.test.ts
Comment thread packages/cli/src/ui/hooks/use-output-style-command.test.ts
Comment thread packages/cli/src/ui/commands/output-style-command.test.ts
Comment thread docs/design/2026-09-01-output-style-picker.md Outdated
Comment thread packages/cli/src/ui/commands/output-style-command.test.ts

@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). Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — unit test phase: zero suites ran in this review (install + builds exhausted the 600s per-call budget on both attempts; no recorded test scope to resume).

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): src/ui/commands/output-style-command.test.tsno such file or directory; src/ui/hooks/use-output-style-command.test.tsno such file or directory; src/ui/hooks/slashCommandProcessor.test.tsno such file or directory; src/ui/hooks/useDialogClose.test.tsno such file or directory; src/ui/components/OutputStyleDialog.test.tsxno such file or directory; and 1 more.

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

  • packages/cli/src/services/BuiltinCommandLoader.ts:163 — [probe] Loader registration of /output-style is untested — deleting the registration leaves every test green (272 tests measured)
  • packages/cli/src/ui/commands/output-style-command.test.ts:27 — [probe] Test suite outcomes depend on ambient QWEN_SYSTEM_MD — 3 tests fail when the variable is exported (reproduced)
中文说明

⚠️ 已从批准降级为评论:CI failing: Test (ubuntu-latest, Node 22.x)。 仅完成部分审查,审查缺口已披露。

未审查:build-and-test — unit test phase: zero suites ran in this review (install + builds exhausted the 600s per-call budget on both attempts; no recorded test scope to resume)。

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

Test Plan(非阻断):src/ui/commands/output-style-command.test.tsno such file or directory; src/ui/hooks/use-output-style-command.test.tsno such file or directory; src/ui/hooks/slashCommandProcessor.test.tsno such file or directory; src/ui/hooks/useDialogClose.test.tsno such file or directory; src/ui/components/OutputStyleDialog.test.tsxno such file or directory; and 1 more。

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

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

@qwen-code-dev-bot qwen-code-dev-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.

Reviewed at head 360211b4.

  • All three round-1 Criticals verified fixed at this head: persistence now writes the scope that actually wins (workspace-owned key keeps the user scope from being shadowed, with the untrusted-workspace write path refusing instead of half-applying), the "saved but no effect" message branches on the real cause (system-prompt replacement incl. QWEN_SYSTEM_MD vs the headless Learning skip) computed from resolveMainSessionOutputStyle, and the description key carries entries in all three strict-parity locales.
  • Write failures now surface via throwOnWriteFailure into the dialog's error path rather than a swallowed success message; the non-interactive no-arg status reports the effective style, not the configured one.
  • 20/20 review threads are resolved; the picker and recording flow read clean. No new Critical issues found.
  • CI on this head has no failures; per the channel convention the call is on the review itself.

@yiliang114 yiliang114 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.

Review passed.

  • Mid-session switching uses the existing seams end to end: config.setOutputStyle + getLlmClient().refreshSystemInstruction() (the style lives in the stable prompt layer, so the rebuild is required and present); persistence happens BEFORE the runtime apply with throwOnWriteFailure, so a failed write never leaves a half-applied state.
  • Scope handling matches the boot-time design: a trusted workspace owning general.outputStyle gets the write to workspace scope (refused when the session policy forbids workspace settings writes), otherwise user scope; --bare/--safe-mode refuse up front; non-interactive/ACP paths report the current style instead of opening a picker.
  • The picker follows the /effort pattern exactly (dialog registration, UIState/UIActions wiring, busy-flag inclusion, Esc cancels with no change); the dialog closes before the apply runs; cancel vs selection are distinct, and the 'saved but no effect' caveats (replaced system prompt, headless Learning skip) reuse the same resolution helper as boot.
  • Unknown names are rejected with the option list; default clears. Comprehensive tests for the command, utils, hook, and dialog.

CI: ubuntu Test failure is an agent-view supervisor timeout flake (worker 'did not report ready before timeout') followed by the 2h job cancel — unrelated to this PR's files; web-shell E2E passed.

@qqqys
qqqys added this pull request to the merge queue Sep 2, 2026
Merged via the queue into QwenLM:main with commit 1772e14 Sep 2, 2026
563 of 564 checks passed

@chiga0 chiga0 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.

No blocking findings.
Approval blockers: none.

Checked / ran (local, linux, node v24):

  • Live-switch path end to end: setOutputStyleLlmClient.refreshSystemInstructiongetMainSessionBaseSystemPromptresolveMainSessionOutputStyle (core/prompts.ts:464) → getCoreSystemPrompt; the per-turn reminder reads the same resolver, so prompt and reminder agree.
  • Settings writer/reader contract: default persists as the literal and resolveOutputStyle (cli config.ts:1291) maps unset/empty/default to no style; unknown names warn instead of locking the user out.
  • Scope/policy logic: trusted workspace owning the key writes back to Workspace scope, untrusted falls back to User, and allowWorkspaceSettingsWrite: false (ACP standalone policy) refuses. The dialog path is gated on executionMode === 'interactive', so the restrictive non-interactive policy is not bypassed.
  • Sibling consistency with /output-language (languageCommand.ts:236); bare/safe-mode guarded in both the command and applyOutputStyleSelection.
  • Ran: new/changed suites 78/78 green · slashCommandProcessor 128/128 · i18n parity (mustTranslateKeys) 20/20.
  • Mutation probes: setOutputStyle/refresh order swap → killed (invocationCallOrder pin) · settings.isTrusted && removal → killed · zh parity translation removal → killed · command-level bare/safe guard removal → survives as an equivalent mutant (the utils-layer guard backstops with an identical refusal; wording-only delta).

Cross-check vs prior ci-bot findings at this head: R1-1/R1-2/R1-3 criticals fixed (workspace-owns scope + isTrusted; correct no-effect reason; zh/zh-TW parity restored) · R1-4's four unpinned behaviours now pinned · R1-5 requiresRestart restored to true · R1-6 write failure surfaced via throwOnWriteFailure · R1-7 no-arg message reports the effective style · R1-8 bare/safe guards + docs row · R1-9 design doc dropped from the PR · R2-1..R2-7 gaps closed in the final commit. Verified by reading at head and by the suites above.

Not covered: the repo-wide unit suite beyond the changed surface · the integration-test legs · Windows/macOS terminal behaviour (TUI-only change, linux evidence).

Context on the CI red behind the latest bot downgrade: Test (ubuntu-latest, Node 22.x) shows one failing test, src/agent-view/supervisor-process.test.ts, in a package this PR does not touch, before the job was cancelled; not attributed to this change.

Reviewed with AI assistance.

qqqys added a commit to qqqys/qwen-code that referenced this pull request Sep 2, 2026
…output-styles

A style is a Markdown file whose body is the prompt section, with an
optional frontmatter (name, description, keep-coding-instructions) that
defaults to the file name, the first body line, and false. The catalog is
built-ins plus the user's files plus, in a trusted workspace, the project's,
with project > user > built-in precedence by case-insensitive name.

The startup resolver, /output-style <name>, and the picker all read the
catalog, so a new file needs no restart. --bare and --safe-mode keep the
built-ins only; invalid files are skipped and logged rather than fatal.

Follows QwenLM#10683 as the custom-style slice it deferred.
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.0.

pull Bot pushed a commit to mcx/qwen-code that referenced this pull request Sep 4, 2026
…output-styles (QwenLM#10761)

* feat: load custom output styles from ~/.qwen/output-styles and .qwen/output-styles

A style is a Markdown file whose body is the prompt section, with an
optional frontmatter (name, description, keep-coding-instructions) that
defaults to the file name, the first body line, and false. The catalog is
built-ins plus the user's files plus, in a trusted workspace, the project's,
with project > user > built-in precedence by case-insensitive name.

The startup resolver, /output-style <name>, and the picker all read the
catalog, so a new file needs no restart. --bare and --safe-mode keep the
built-ins only; invalid files are skipped and logged rather than fatal.

Follows QwenLM#10683 as the custom-style slice it deferred.

* fix(output-style): stop a style file reading anything but itself

Two ways a style file could carry content it was never meant to.

A style file's body goes into the system prompt verbatim, and the loader
followed links with only an `fs.stat`. A repo can commit
`.qwen/output-styles/notes.md -> ~/.aws/credentials` plus a `.qwen/settings.json`
naming it; folder trust is off by default, so `git clone && qwen` treats the
workspace as trusted, `stat.isFile()` reports on the *target*, and the secret
is embedded under `# Output Style: notes` with no tool call and no approval.
A hard link (`nlink > 1`) and a symlinked ancestor (a checked-in
`.qwen -> /outside`, which a final-component check cannot see) reach the same
place.

This mirrors `readLoopTaskFile`, which guards the identical sink. A project
file is read with `lstat`, so a link is not a regular file and never reaches
the read -- confinement alone would wave through `notes.md -> .env`, whose
target is inside the root. A user file may be a link, because
`~/.qwen/output-styles/x.md -> ~/dotfiles/x.md` is an ordinary setup, but its
canonical path must stay inside the home root. Both refuse `nlink > 1` and
both confine the realpath.

Separately, the description reached the terminal un-sanitized while the
sibling `name` from the same untrusted file was refused for exactly those
characters, and a declared description had no length bound: an OSC-8 hyperlink
made a picker row link somewhere it did not say, and U+202E reordered the row
so the `(project)` marker read as something else. Both description sources now
go through `stripAnsiAndControl` plus a `\p{Cf}` strip, collapse to one line,
and share the cap the derived description already used.

Each guard is pinned by a case that goes red without it, including the
in-workspace symlink that distinguishes `lstat` from confinement.

* fix(output-style): key the headless Learning rule on the built-in, not the name

The docs this PR adds tell users a file "overrides a built-in of that name",
so a user customizing Learning ships `~/.qwen/output-styles/Learning.md`. The
lowercase dedupe key shadows the built-in and the name keeps the basename's
capital L, so `resolveEffectiveOutputStyle`'s `name === 'Learning'` check
fires on the user's own prompt: `qwen -p` then carries no output style at all
-- no prompt section, no turn reminder -- and `warnAboutOutputStyle` stays
quiet because the name resolved, so it is a silent no-style run. Renaming the
file `learning.md` shadows the identical built-in and *is* applied, so two
spellings of one name behaved differently in a module that dedupes and looks
names up case-insensitively everywhere else.

The rule exists because the built-in Learning prompt tells the model to stop
and wait for a reply a headless run cannot send. A custom file that happens to
take the name carries no such instruction, so the check keys on
`source === 'built-in'` as well.

* fix(output-style): stop a dismissed picker re-opening behind the user

`openOutputStyleDialog` suspends on `loadSessionOutputStyles` and then sets
the dialog open. A dismissal landing in that window leaves the continuation
queued, so the dialog the user just closed re-opens and the next Enter is
captured by `RadioButtonSelect`, which writes `general.outputStyle` with
`{ throwOnWriteFailure: true }`. An open generation, retired both by a newer
open and by a selection, makes the stale continuation a no-op.

The defect had no in-suite witness because this diff deleted
`act(() => result.current.openOutputStyleDialog())` from `clears the style
when "default" is chosen` rather than adapting it with the siblings'
`waitFor`. Its closing `expect(isOutputStyleDialogOpen).toBe(false)` then read
the `useState(false)` initial value and passed for any implementation,
including one that never closes. The open is restored with the wait, and a new
case dismisses an open and then resolves its load, asserting the dialog stays
closed.

* fix(opentui): give the output-style picker the same catalog as ink

Startup style resolution is renderer-independent, so a custom style can be
active under OpenTUI via `--output-style`, `general.outputStyle`, or the
renderer-agnostic `/output-style <name>`. The dialog built its list from
`BUILT_IN_OUTPUT_STYLES` alone, so the active style was not in it,
`findIndex` returned `-1`, and `Math.max(0, -1)` highlighted `default` --
contradicting the comment two lines below claiming index 0 tells the truth.
One Enter then applied `undefined` and persisted `"outputStyle": "default"`
with `{ throwOnWriteFailure: true }`, clearing the user's own setting. The
docs line this PR adds, that custom styles appear in the picker labelled with
their source, was false on this renderer.

The dialog now loads through `loadSessionOutputStyles`, which carries the ink
path's trust gate, and labels non-built-ins with their source the way
`describe()` does. `useState` runs its initialiser once, so the pre-selection
is re-derived when the catalog arrives -- otherwise the `-1 -> 0` clamp would
survive the fix.

The test stubs the catalog load so it never reads the developer's own
`~/.qwen/output-styles`, and asserts a `user` style renders as a selectable
row and is the pre-selected one.

* fix(output-style): make picker loading non-interactive

* fix(opentui): keep the output-style picker steady while it is open

The catalog effect depended on the `onClose` and `notify` props, which the
mount site rebuilds as inline closures on every render, so any shell
re-render tore down the settled load and re-read both style directories.
The reload landed a new array, the selection was re-derived, and the user's
arrow-key navigation was discarded -- Enter then applied the previously
active style. Keep the callbacks in refs and depend only on `config`.

The catalog is also re-read on every open and skips a file it cannot parse,
so the active style can be missing from it while the session still runs it.
Snapping to index 0 marked `default` as the active style and one Enter
persisted that over the user's setting; list the live definition instead,
matching membership case-insensitively the way the catalog dedupes.

* fix(output-style): read a style file the way a prompt should be read

The loader took plausible Markdown files and turned them into something
the author did not write. A `---` rule around prose was parsed as
frontmatter and its text vanished from the prompt, while a fence with a
trailing space was not frontmatter at all and its raw YAML became the
prompt; an HTML comment meant for a teammate was fed to the model, and a
comment-only template file loaded as a selectable style; a UTF-16 file --
what PowerShell's `>` and Notepad write -- decoded to NUL-riddled
mojibake and still reached the system prompt; the size bound was 41x the
house bound for this sink and was checked against a `stat` the file could
outgrow between the check and the read.

A file that shadows a built-in also inherited `keep-coding-instructions:
false`, so customizing the wording of `concise.md` silently deleted the
verification and faithful-reporting guidance, and a declared `True` was
read as false whenever an unrelated frontmatter line made the YAML parser
fall back to its simple mode.

Alongside those: the file-name-derived name is trimmed, so a stray
trailing space no longer makes an entry that is listed but unselectable
and ` default.md` no longer bypasses the reserved name; a non-string YAML
`name` falls back to the file name instead of throwing an opaque
TypeError or loading as `a,b`; and the home-directory guard compares the
directories that would actually be read, so a relocated `QWEN_HOME`
neither hides the project level nor relabels the user's own files.

`stripHtmlComments` moves to `textUtils` next to `normalizeContent`, the
home the rules loader's copy should always have had.

* fix(output-style): keep a resolved style inside the session it belongs to

Two consumers took the resolved style further than it was meant to go.
Project trust was enforced only where the catalog is read, so a
checked-in style kept shaping every turn after the workspace lost trust
mid-session -- the IDE companion changes that verdict in place, with no
restart -- while the picker had already stopped listing it. The gate now
sits in the resolver the prompt and the per-turn reminder share, so both
follow the flip.

Arena peers were handed `getOutputStyle()` raw, so a peer inherited a
style even when the main session's prompt was replaced, and a style with
`keepCodingInstructions: false` stripped the software-engineering
guidance -- verification and faithful reporting included -- from agents
whose whole job is to produce a diff.

The trust mock in the config tests answered the same way whatever it was
asked, and the unknown-style warning's "Available styles" list had no
test at all; both are now pinned.

* fix(output-style): say what the setting takes, and where a choice lands

Four surfaces described the value `--output-style` and
`general.outputStyle` accept: two still listed only the four built-ins,
and two named the style *file* where the code matches the declared
frontmatter `name` -- so a user with `review-v2.md` declaring `name:
Reviewer` typed `review-v2`, following the editor hover attached to the
very file being edited, and got an unknown-style warning on every
startup.

Selecting a style also wrote the name wherever the key already lived,
without saying so: a project style's name into user settings, where no
other project can resolve it, or a personal style's name into the shared
checked-in workspace file, where every teammate and CI run warns and runs
unstyled. The scope stays where it is -- moving it would be shadowed by
the workspace value -- and the confirmation now names the mismatch.

The ink picker matched the active style name exactly while every other
lookup is case-insensitive, and an active style the re-read catalog no
longer carries fell back to `default`, where one Enter persisted it over
the user's setting.

Also restores the design doc and the E2E plan dropped by an earlier fix
commit, with the corrections their reviews asked for: the picker resolves
against the open-time snapshot, so a file deleted after the picker opened
still applies and its persisted name warns at the next startup -- the
opposite of what the bullet claimed -- and the plan's baseline, width,
trust setup and restore steps are now executable and falsifiable.

* fix(cli): explain inactive project output styles

---------

Co-authored-by: 易良 <1204183885@qq.com>
Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants