Skip to content

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

Merged
qqqys merged 17 commits into
QwenLM:mainfrom
qqqys:feat/output-style-files
Sep 4, 2026
Merged

qqqys merged 17 commits into
QwenLM:mainfrom
qqqys:feat/output-style-files

Conversation

@qqqys

@qqqys qqqys commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This lets users define their own output styles as Markdown files. A file in ~/.qwen/output-styles/ is a user style; a file in the project's .qwen/output-styles/ is a project style. The body is the style's prompt section, and an optional YAML frontmatter carries name, description, and keep-coding-instructions, defaulting to the file name, the first body line, and false. A bare Markdown file is therefore already a valid style.

Core gains loadOutputStyleCatalog, which returns the built-ins plus the user's and (when a project root is given) the project's files, deduplicated by case-insensitive name with project > user > built-in precedence, in picker order. parseOutputStyleFile and loadOutputStylesFromDir are exported separately so the extension slice can reuse them with source: 'extension'.

The CLI reads the catalog in three places: at startup to resolve general.outputStyle or --output-style, when /output-style <name> is given, and when the picker opens. Custom entries appear after the built-ins with their source in the label. The picker resolves the selection against the list it displayed, so a file removed between open and select gives a clear error instead of a silent no-op. Project files are read only from a trusted workspace; --bare and --safe-mode keep the built-ins only. An invalid file (empty body, reserved or malformed name, over 1 MiB) is skipped and logged, and the other files still load.

Documentation gains a "Custom styles" section in output-styles.md, the settings and headless references mention custom names, the VS Code settings schema is regenerated, and the design decisions are recorded in docs/design/2026-09-02-output-style-files.md.

Why it's needed

#9565 shipped built-in styles, #10283 made them selectable at startup, and #10683 added the /output-style picker. All three left OutputStyleSource's user and project members unpopulated, so the only styles a user could pick were the four built-ins. This slice makes the style set user-extensible with the same file shape other CLI agents use, so one file works across tools, while keeping the loader small: two directories, top level only, no watcher.

Reviewer Test Plan

How to verify

Create ~/.qwen/output-styles/reviewer.md with a name: Reviewer frontmatter and a body, .qwen/output-styles/haiku.md with just a body, and .qwen/output-styles/broken.md with name: default. Run npm run dev and /output-style: the picker should list the four built-ins, then Reviewer … (user) and haiku … (project), and no broken entry. Select haiku and ask a question; the next answer should be a haiku and ~/.qwen/settings.json should carry "outputStyle": "haiku". Confirm /output-style REVIEWER applies case-insensitively, that a file added while the session is open shows up on the next /output-style without a restart, that an unknown name lists the custom styles among the choices, and that after a restart the persisted custom style applies from startup. The full plan is in .qwen/e2e-tests/output-style-files.md.

Focused verification:

# packages/core
npx vitest run src/core/output-style-files.test.ts src/core/output-styles.test.ts
# packages/cli
npx vitest run src/ui/commands/output-style-command.test.ts src/ui/hooks/use-output-style-command.test.ts src/ui/components/OutputStyleDialog.test.tsx src/config/settingsSchema.test.ts src/ui/hooks/slashCommandProcessor.test.ts src/ui/hooks/useDialogClose.test.ts src/services/BuiltinCommandLoader.test.ts src/i18n/mustTranslateKeys.test.ts
npx vitest run src/config/config.test.ts -t "output style"

Result: core 2 files, 41 tests passed; CLI 8 files, 247 tests passed, plus 28 config output-style tests passed. ESLint and Prettier are clean on the changed files. tsc --noEmit in packages/cli reports one error that is already present on main (ui/opentui/commands-registry.ts does not route the output-style dialog kind); that is a separate OpenTUI parity gap and is not touched here.

Evidence (Before & After)

Before: /output-style offered only default and the four built-ins; a custom name in general.outputStyle warned and fell back to the default style.

After, with the fixtures above, from a real npm run dev session:

│ > Output Style (applies now and persists to settings)                       │
│                                                                             │
│ › 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 ov…│
│   4. Explanatory — Explains implementation choices and codebase patterns al…│
│   5. Learning — Hands the user small, meaningful pieces of code to write, t…│
│   6. Reviewer — Reviews code and reports findings without editing anything (user) │
│   7. haiku — Answer every question as a haiku. Keep it to three lines. (project)  │
●︎ Output style set to haiku.
> what does scripts/dev.js do?
◆︎ Runs CLI from source —
  tsx, no build step needed;
  edits show up live, fast.
> /output-style REVIEWER
●︎ Output style set to Reviewer.
> /output-style nope
✕ Unknown output style "nope". Choose one of: Concise, Proactive, Explanatory, Learning, Reviewer, haiku,
  terse, or "default" for no style.

terse above is a file added while the session was open; it appeared on the next /output-style without a restart. After /quit and a fresh npm run dev, the first question was answered as a haiku with no command issued.

Tested on

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

Environment (optional)

Node 22; npm run dev for the interactive session; focused Vitest runs in packages/core and packages/cli.

Risk & Scope

  • Main risk or tradeoff: a checked-in project style is a prompt, so it is read only from a trusted workspace, the same line the workspace settings.json already draws. A custom file may shadow a built-in name, mirroring how project skills shadow bundled ones.
  • Not validated / out of scope: extension-bundled styles (source: 'extension', the outputStyles manifest field the Claude-plugin converter currently warns about), a policy/managed level, additional-directory scans, a settings-file watcher, and the OpenTUI route for the output-style dialog kind.
  • Breaking changes / migration notes: none. resolveOutputStyle gains an optional third parameter that defaults to the built-ins.

Linked Issues

Follows #9565, #10282, #10283, and #10683.

中文说明

这个 PR 做了什么

本 PR 允许用户用 Markdown 文件定义自己的输出风格。~/.qwen/output-styles/ 下的文件是用户级风格,项目 .qwen/output-styles/ 下的是项目级风格。文件正文就是风格的提示词段落,可选的 YAML frontmatter 携带 namedescriptionkeep-coding-instructions,缺省分别为文件名、正文首行和 false,因此一个纯 Markdown 文件本身就是合法风格。

core 新增 loadOutputStyleCatalog:返回内置风格加用户文件、加(给定项目根时)项目文件,按大小写不敏感的名称去重,优先级 project > user > built-in,输出顺序即选择器顺序。parseOutputStyleFileloadOutputStylesFromDir 单独导出,便于扩展切片以 source: 'extension' 复用。

CLI 在三处读取目录:启动时解析 general.outputStyle--output-style/output-style <name> 时,以及打开选择器时。自定义条目排在内置之后并在标签中标注来源。选择器按打开时展示的列表解析选择,因此在打开与选择之间被删除的文件会给出明确错误而不是静默无效。项目文件只在可信工作区读取;--bare--safe-mode 只保留内置风格。无效文件(空正文、保留或非法名称、超过 1 MiB)会被跳过并记入调试日志,其余文件照常加载。

文档方面,output-styles.md 新增「Custom styles」章节,settings 与 headless 参考提到自定义名称,VS Code 设置 schema 已重新生成,设计取舍记录在 docs/design/2026-09-02-output-style-files.md

为什么需要它

#9565 提供内置风格,#10283 让启动时可选,#10683 增加了 /output-style 选择器,但三者都没有填充 OutputStyleSource 中的 userproject,用户能选的仍只有四个内置风格。本切片让风格集合可由用户扩展,文件格式与其他 CLI 代理一致,一份文件多处可用,同时把加载器保持得很小:两个目录、仅顶层、无监听。

评审者测试计划

如何验证

创建 ~/.qwen/output-styles/reviewer.md(frontmatter 含 name: Reviewer)、.qwen/output-styles/haiku.md(仅正文)和 .qwen/output-styles/broken.mdname: default)。运行 npm run dev 并执行 /output-style:选择器应列出四个内置风格,随后是 Reviewer … (user)haiku … (project),且没有 broken。选择 haiku 后提问,下一条回答应为俳句,~/.qwen/settings.json 中应有 "outputStyle": "haiku"。确认 /output-style REVIEWER 大小写不敏感;会话中新增文件后再次 /output-style 无需重启即可看到;未知名称的报错列表包含自定义风格;重启后已持久化的自定义风格从启动起生效。完整计划见 .qwen/e2e-tests/output-style-files.md

上方 focused Vitest 命令:core 2 个文件 41 项测试通过;CLI 8 个文件 247 项测试通过,另有 config 的 28 项 output-style 测试通过。改动文件的 ESLint 与 Prettier 均干净。packages/clitsc --noEmit 有一处 main 上已存在的错误(ui/opentui/commands-registry.ts 未路由 output-style 对话框类型),属于独立的 OpenTUI 对齐缺口,本 PR 不涉及。

证据(Before & After)

Before:/output-style 只提供 default 与四个内置风格;general.outputStyle 写自定义名称会警告并回退默认风格。

After:见上方真实会话记录。选择 haiku 后下一条回答即为俳句;REVIEWER 大小写不敏感生效;会话中新增的 terse 无需重启即出现在列表与报错提示中;重启后首个回答仍为俳句。

测试环境

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

风险与影响范围

  • 主要风险或权衡:随仓库提交的项目风格文件本质是提示词,因此只在可信工作区读取,与工作区 settings.json 的界线一致。自定义文件可以遮蔽内置名称,与项目级 skill 遮蔽内置 skill 的行为一致。
  • 未验证 / 范围外:扩展捆绑的风格(source: 'extension',即 Claude 插件转换器目前告警的 outputStyles 字段)、策略/托管级目录、additional directories 扫描、设置文件监听,以及 OpenTUI 对 output-style 对话框类型的路由。
  • 破坏性变更 / 迁移说明:无。resolveOutputStyle 新增可选的第三个参数,缺省为内置风格。

关联 Issue

承接 #9565#10282#10283#10683

…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

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

Copy link
Copy Markdown
Collaborator

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all sections present, bilingual body, reviewer test plan with before/after.

Problem: this is an observed gap, not a theoretical one. The output-style chain already shipped — #9565 (prompt layer), #10282 (per-turn reminder), #10283 (startup selection), #10683 (/output-style picker) — and OutputStyleSource on main still carries the comment "Only built-in is populated today; the remaining sources exist so that user/project markdown files … can be registered later." This PR is the slice that comment was written for: populating user and project.

Direction: aligned. Claude Code's changelog shows custom/project output styles are an established cross-tool expectation — including the keep-coding-instructions frontmatter key this PR mirrors, and fixes for "custom, project, and plugin output styles drifting back to the default voice". Worth noting Claude Code has since deprecated its /output-style command in favor of /config with the style fixed at session start (prompt-caching reasons); qwen-code already shipped the mid-session switcher in #10683, so building on that surface is the consistent call here — but that upstream direction is worth keeping in mind for later slices.

Size: touches core paths (packages/core/src/core/, packages/cli/src/config/). Breakdown: 474 production logic lines (269 core + 205 CLI), 496 test lines, 207 docs, 2 generated/schema. Under the 500-line maintainer-awareness line; feat type, so no size gate applies.

Approach: scope feels right and matches what I'd have done independently: a core catalog loader (parseOutputStyleFile / loadOutputStylesFromDir / loadOutputStyleCatalog) plus wiring at the three existing resolution points (startup setting, /output-style <name>, picker). Two directories, top level only, no watcher, extension level explicitly deferred to the next slice — nothing I'd cut. The trust line (project files only from a trusted workspace, built-ins only under --bare/--safe-mode) matches what workspace settings.json already draws, and the precedence (project > user > built-in) mirrors SkillManager.

Risk: no elevated-risk path signals (no matches against the revert-correlated paths).

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 各节齐全,中英双语,附带评审者测试计划和 before/after。

问题: 这是已观测到的缺口,不是理论问题。输出风格链已经落地 —— #9565(提示词层)、#10282(每轮提醒)、#10283(启动时选择)、#10683/output-style 选择器)—— 而 main 上的 OutputStyleSource 仍写着 "Only built-in is populated today; the remaining sources exist so that user/project markdown files … can be registered later"。本 PR 正是为填补这段注释而生的切片:填充 userproject

方向: 对齐。Claude Code 的 changelog 显示自定义/项目级输出风格已是跨工具的既有预期 —— 包括本 PR 沿用的 keep-coding-instructions frontmatter 键,以及针对 "custom, project, and plugin output styles drifting back to the default voice" 的修复。值得注意的是 Claude Code 后来弃用了 /output-style 命令、改用 /config 并在会话开始固定风格(出于提示词缓存考虑);qwen-code 已在 #10683 提供了会话中切换器,基于现有界面继续建设是一致的选择 —— 但上游的这个方向值得在后续切片中留意。

规模: 触及核心路径(packages/core/src/core/packages/cli/src/config/)。构成:474 行生产逻辑(core 269 + CLI 205)、496 行测试、207 行文档、2 行生成/schema。低于 500 行维护者关注线;feat 类型,不触发规模门禁。

方案: 范围合理,与我独立设想的做法一致:core 目录加载器(parseOutputStyleFile / loadOutputStylesFromDir / loadOutputStyleCatalog)+ 在三个既有解析点接线(启动设置、/output-style <name>、选择器)。两个目录、仅顶层、无监听,扩展级明确留给下一个切片 —— 没有可砍的部分。信任界线(项目文件只在可信工作区读取,--bare/--safe-mode 只保留内置)与工作区 settings.json 已有的界线一致,优先级(project > user > built-in)与 SkillManager 相同。

风险: 无升级风险信号(未命中与回滚相关的高风险路径)。

进入代码审查 🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code review

I wrote my independent proposal before reading the diff — a core catalog loader plus wiring at the three existing resolution points, trust-gated project reads — and this PR lands essentially that shape, so there was no simpler path it missed. What's here is careful:

  • Consumers are all accounted for. resolveOutputStyleChoice has exactly two call sites (the command and the picker hook) and both pass the loaded catalog; resolveOutputStyle is only called from loadCliConfig; getBuiltInOutputStyle stays exported for its remaining consumers (ACP sessions, arena, core tests). Nothing is left calling the old signatures.
  • The trust line is drawn consistently at all three resolution points — startup (isWorkspaceTrustedprojectRoot or undefined), /output-style <name> (config.isTrustedFolder()), and the picker (loadSessionOutputStyles). --bare/--safe-mode never read the catalog, and the command's existing guard refuses before any disk read.
  • Untrusted input is handled. Style names are validated (empty, >64 chars, control/format characters, reserved default) before they can reach the system-prompt heading or the picker; files cap at 1 MiB; a bad file is skipped with a debug-log line instead of hiding its neighbours; frontmatter goes through the hardened yaml-parser (null-stripping, __proto__ protection), which never throws — malformed frontmatter degrades to defaults rather than crashing the load.
  • The dedupe and precedence are right (project > user > built-in, case-insensitive, shadowed entries removed entirely, picker order preserved) and covered by tests, as are the edge cases I'd ask about: CRLF/BOM input, home-directory-as-project-root, same-name files, missing directory.
  • A nice detail: the picker resolves the selection against the list it actually displayed (choicesRef), so a file deleted between open and select errors cleanly instead of silently no-op'ing, and a catalog read failure falls back to opening with the built-ins.

No critical blockers, no convention violations (kebab-case filenames, colocated tests, ESM, no any). One non-blocking observation: the repo now has four inline copies of the same frontmatter regex (rulesDiscovery, subagent-manager, claude-converter, and this one). This PR follows the established pattern rather than adding a fifth divergence, and extracting a shared helper across four different schemas is a separate cleanup, not something this PR should carry.

Files changed (22)
File What changed
packages/core/src/core/output-style-files.ts The new loader: frontmatter parse, name/body validation, per-dir load with skip-and-log, catalog with precedence dedupe
packages/core/src/core/output-style-files.test.ts 259 lines covering parsing, validation, dedupe, precedence, trust omission
packages/core/src/index.ts Exports the new module
packages/cli/src/config/config.ts Loads the catalog at startup (bare/safe/trust gated); resolveOutputStyle accepts the selectable list
packages/cli/src/config/config.test.ts Custom selection, untrusted workspace, bare/safe-mode tests
packages/cli/src/config/settingsSchema.ts outputStyle description mentions custom style files
packages/cli/src/ui/commands/output-style-command.ts Non-interactive listing and name resolution read the catalog
packages/cli/src/ui/commands/output-style-command.test.ts Custom-name, case-insensitivity, trust gating tests
packages/cli/src/ui/commands/output-style-utils.ts loadSessionOutputStyles helper; resolveOutputStyleChoice takes a catalog
packages/cli/src/ui/hooks/use-output-style-command.ts Loads catalog when the picker opens; selection resolves against the displayed list
packages/cli/src/ui/hooks/use-output-style-command.test.ts Async open, failure fallback, custom select, unknown-name tests
packages/cli/src/ui/components/OutputStyleDialog.tsx Renders a given style list; custom entries labelled with source
packages/cli/src/ui/components/OutputStyleDialog.test.tsx Custom entry rendering and pre-selection test
packages/cli/src/ui/components/DialogManager.tsx Passes the loaded choices into the dialog
packages/cli/src/ui/AppContainer.tsx Threads outputStyleChoices through UI state
packages/cli/src/ui/contexts/UIStateContext.tsx Adds outputStyleChoices to UIState
packages/vscode-ide-companion/schemas/settings.schema.json Regenerated description
docs/users/features/output-styles.md New Custom styles section: directories, frontmatter fields, precedence
docs/users/configuration/settings.md general.outputStyle description updated (table rewrap)
docs/users/features/headless.md --output-style reference includes custom names
docs/design/2026-09-02-output-style-files.md Design decisions recorded, incl. explicit out-of-scope list
.qwen/e2e-tests/output-style-files.md E2E plan: picker, apply, persist, safe-mode and untrusted refusals

Test evidence — the PR's own CI

The table below reflects the PR's checks at the reviewed commit, fetched via the API; I have not run any of the PR's code or tests (static review only, per triage rules). CI is not settled yet: the ubuntu unit suite is still running, and it is the one to watch. The macOS/Windows test jobs and the second integration lane are skipped on this fork PR, so platform coverage comes from CI only via the green Desktop Shell checks.

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

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

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

The one red check is pre-existing on main, not caused by this PR. Integration Tests (no-AK, No Sandbox) fails during npm run build --workspace=packages/cli with src/ui/opentui/commands-registry.ts(177,13): error TS2322: Type '"output-style"' is not assignable to type 'never'. Verified statically against the base: on main the OpenDialogActionReturn.dialog union already contains 'output-style' (added by #10683), while the OpenTUI registry's exhaustiveness switch — activated by #10739, the current base commit — has no case for it. This PR touches neither file. The author disclosed this in the PR body. It does mean the PR cannot get a fully green board until main is fixed (or the PR rebases after that fix), and the same gap means the OpenTUI backend can't route /output-style — out of scope here per the author, but worth a follow-up issue.

Sandboxed verification would settle the behavioural claim: @qwen-code /tmux — that /output-style actually lists files from ~/.qwen/output-styles and the project's .qwen/output-styles, and that selecting one reshapes the next answer and persists across restart, is asserted by the author's session transcript and unit tests but not exercised live here; the author has write access, so both /tmux and an A/B @qwen-code /verify are available lanes.

中文说明

代码审查

读 diff 之前我先独立写了方案 —— core 目录加载器 + 三个既有解析点接线 + 信任门控的项目读取 —— 本 PR 的形状与之一致,没有更简的漏网路径。实现很细致:

  • 所有消费者都已核对。 resolveOutputStyleChoice 只有两个调用点(命令与选择器 hook),均已传入目录;resolveOutputStyle 只被 loadCliConfig 调用;getBuiltInOutputStyle 仍导出给其余消费者(ACP 会话、arena、core 测试)。没有残留旧签名的调用。
  • 信任界线在三个解析点画得一致 —— 启动(isWorkspaceTrustedprojectRootundefined)、/output-style <name>config.isTrustedFolder())、选择器(loadSessionOutputStyles)。--bare/--safe-mode 从不读取目录,命令的既有守卫在任何磁盘读取之前先行拒绝。
  • 不可信输入已处理。 风格名在能进入系统提示词标题或选择器之前经过校验(空名、超过 64 字符、控制/格式字符、保留名 default);文件上限 1 MiB;坏文件被跳过并记调试日志,不会连累其他文件;frontmatter 走加固过的 yaml-parser(去 null、防 __proto__),且永不抛错 —— 畸形 frontmatter 退化为默认值而不是崩溃。
  • 去重与优先级正确(project > user > built-in,大小写不敏感,被遮蔽条目完全移除,选择器顺序保留),且有测试覆盖;我在意的边界也都有:CRLF/BOM、项目根恰为家目录、同名文件、目录不存在。
  • 一个亮点: 选择器按打开时展示的列表解析选择(choicesRef),打开与选择之间文件被删除会得到明确错误而非静默无效;目录读取失败时回退为只展示内置项。

无关键阻塞项,无约定违规(kebab-case 文件名、测试与源码同目录、ESM、无 any)。一个非阻塞观察:仓库里现在有四处同样的 frontmatter 正则内联副本(rulesDiscoverysubagent-managerclaude-converter 和本文件)。本 PR 沿用既有模式而非新增第五种分歧;抽取共享 helper 涉及四种不同 schema,属于另一项清理,不应由本 PR 承担。

测试证据

上表来自评审提交上的 PR 自身 CI(经 API 获取);按 triage 规则,本次为纯静态审查,未运行任何 PR 代码或测试。CI 尚未定型:ubuntu 单元测试仍在运行,是最需要关注的一项。本 fork PR 上 macOS/Windows 测试与第二条集成通道被跳过,平台覆盖仅来自两个绿色的 Desktop Shell 检查。

唯一红色检查main 上的既有问题,并非本 PR 引起Integration Tests (no-AK, No Sandbox) 在构建 packages/cli 时失败于 commands-registry.ts(177,13): TS2322。已基于基线静态核实:mainOpenDialogActionReturn.dialog 联合类型已含 'output-style'#10683 加入),而 OpenTUI 注册表的穷尽 switch(由 #10739 激活,即当前基线提交)缺少该分支;本 PR 未触及这两个文件,作者在 PR 正文中也已披露。这意味着在 main 修复(或修复后 rebase)之前,本 PR 拿不到全绿;同一缺口也意味着 OpenTUI 后端无法路由 /output-style —— 作者已声明超出本 PR 范围,建议跟进一个 issue。

沙箱验证可以坐实行为主张:@qwen-code /tmux —— /output-style 是否真的列出 ~/.qwen/output-styles 与项目 .qwen/output-styles 中的文件、选择后下一条回答是否真的改变风格并跨重启持久化,目前由作者的会话记录与单元测试支撑,本次未做活体验证;作者有写权限,/tmux 与 A/B @qwen-code /verify 两条通道均可用。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean review across every stage; the only reservations are external to the diff (a red board inherited from main, and live-TUI behaviour not yet exercised in a sandboxed lane).

Stepping back: this is the natural fourth slice of a shipped feature chain, filling an extension point that main's own code comments say was left for exactly this work. The implementation matches the independent proposal I wrote before reading the diff — I found no simpler path it missed — and it carries the habits that make a fork PR easy to land: a design doc that records what was deliberately left out, an E2E plan, trust-gating drawn at the same line as workspace settings, validation before anything reaches the prompt, and tests for the edge cases rather than just the happy path. Every change in the diff serves the stated goal; the docs and design file are repo convention, not bloat. The one upstream signal worth keeping in mind — Claude Code deprecating its /output-style command in favour of session-start-fixed styles — doesn't change the call here, since qwen-code already shipped the mid-session switcher; it's context for the next slice.

What keeps this at 4 rather than 5: I can't attest to the behaviour end-to-end from a static review. The author's transcript and the 496 lines of new tests are good evidence, but the ubuntu unit suite is still running on this commit, and the integration lane is red — verified as a pre-existing main break (the OpenTUI registry missing the output-style dialog route), not this PR's doing. Practically, that means the deferred approval below can only fire once main is fixed and this commit's checks land green (a rebase after the fix will do it); until then a maintainer can also run @qwen-code /tmux or @qwen-code /verify to settle the live-behaviour question directly.

Approval deferred until CI lands green on a8186c41abb508b2f3dbdd0566daee89562e365e.

中文说明

回顾整体:这是已落地功能链上顺理成章的第四个切片,填补的正是 main 代码注释中明确预留的扩展点。实现与我在读 diff 之前独立写下的方案一致 —— 没找到它漏掉的更简路径 —— 并具备让 fork PR 容易合入的习惯:记录"刻意不做什么"的设计文档、E2E 计划、与工作区设置同一界线的信任门控、任何内容进入提示词之前的校验,以及针对边界情况而非仅正常路径的测试。diff 中每一处改动都服务于既定目标;文档与设计文件是仓库约定,不是膨胀。一个值得留意的上游信号 —— Claude Code 弃用 /output-style 命令、改为会话开始固定风格 —— 不改变此处的判断,因为 qwen-code 已提供会话中切换器;它是下一个切片的背景。

之所以是 4 分而非 5 分:静态审查无法为端到端行为背书。作者的会话记录与 496 行新测试是很好的证据,但该提交上的 ubuntu 单元测试仍在运行,集成通道是红的 —— 已核实为 main 的既有损坏(OpenTUI 注册表缺少 output-style 对话框路由),并非本 PR 造成。实际影响是:下面的延迟批准只能等 main 修复、本提交的检查转绿后才会触发(修复后 rebase 即可);在此之前,维护者也可以运行 @qwen-code /tmux@qwen-code /verify 直接坐实实时行为问题。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

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

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none of my scope was cut short — I read all 367 assigned diff lines (511-877) un-truncated and the full post-change source of every production file in it. I did….

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

Test Plan (not a blocker): src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more.

⚠️ 11 finding(s) were deferred without a posture licence — no posture is engaged on round 1 and no age reference exists. They are listed in this body when it has room for them, and always in the terminal report and this run's findings artifact; this verdict is capped either way: findings may be under-posted this round.

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

  • docs/design/2026-09-02-output-style-files.md:50 — [probe] design doc + PR description claim a clear error when a style file is deleted between open and select; the code silently applies the cached definition and persists an unresolvable nam…
  • packages/cli/src/config/config.ts:1374 — [probe] the Unknown output style warning states something false when the catalog was deliberately narrowed by --bare/--safe-mode or an untrusted workspace, and names neither cause
  • packages/cli/src/ui/commands/output-style-utils.ts:35 — [probe] the picker shows the freshly read definition as current while the session runs the startup one, with no re-apply cue after the active file is edited
  • packages/cli/src/ui/commands/output-style-utils.ts:59 — [probe] getBuiltInOutputStyle left exported with no production caller; two public resolvers now differ only by scope
  • packages/cli/src/ui/components/OutputStyleDialog.tsx:41 — [probe] the unbounded picker list windows at 10 rows with no scroll affordance (neither showScrollArrows nor maxItemsToShow is passed)
  • packages/cli/src/ui/components/OutputStyleDialog.tsx:50 — [probe] the (user)/(project) source marker is appended last in a wrap=truncate label, so it is cut off for any description over ~50 chars
  • packages/cli/src/ui/hooks/use-output-style-command.test.ts:89 — [probe] the only degraded-catalog test fixtures a rejection the loader cannot produce, so a guard placed inside the catch passes green and fixes nothing
  • packages/core/src/core/output-style-files.ts:59 — [probe] the filename-derived name is not trimmed, so a trailing space silently resolves to the built-in or to nothing
  • packages/core/src/core/output-style-files.ts:62 — [probe] the declared name is coerced with String() and no type check; a YAML mapping throws with no reachable diagnostic and an array becomes a comma-joined name
  • packages/core/src/core/output-style-files.ts:128 — [probe] deriveDescription emphasis strip deletes content characters (snake_case to snakecase, __init__ to init, *.md to .md)
  • packages/core/src/core/output-style-files.ts:227 — [probe] the home-directory guard compares roots not the two directories, so it mis-fires on an empty homedir and on QWEN_HOME relocation
中文说明

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

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

未审查:build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour。

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)"none of my scope was cut short — I read all 367 assigned diff lines (511-877) un-truncated and the full post-change source of every production file in it. I did…

未审查:反向审计——在 5 轮的反审轮数上限内未收敛。

Test Plan(非阻断):src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more。

⚠️ 11 条发现在姿态未授权的情况下被延后——no posture is engaged on round 1 and no age reference exists。正文空间允许时会列出清单,完整内容始终在终端报告与本次运行的 findings 工件中;无论如何本判定已被限制:本轮发现可能未被完整发布。

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

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

Comment thread packages/cli/src/ui/components/DialogManager.tsx
Comment thread packages/cli/src/ui/hooks/use-output-style-command.test.ts
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts
@qqqys

qqqys commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover from 1

@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 Sep 3, 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). This window's round counter starts at 1 (the rounds this PR spent in review before takeover), so the Critical-only brake engages after 4 more change-producing round(s) instead of a full fresh 5. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。本窗口轮次计数从 1 起算(即本 PR 托管前已进行的评审轮数),因此再经过 4 个产生改动的轮次即进入 Critical-only,而非重新计满 5 轮。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

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

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

  • R1-61 seven unusable E2E plan steps — still standing; already reported (comment 3914106020)
  • R1-21 keepCodingInstructions:false reachable via config.getOutputStyle/arena prompts — still standing; already reported (comment 3914106031)
  • R1-53 project trust gate enforced only on the establishing side — still standing; already reported (comment 3914106042)
  • R1-62 four stale surfaces describing the outputStyle value — still standing; already reported (comment 3914106056)
  • R1-8 persist scope chosen by key ownership, ignoring style source — still standing; already reported (comment 3914106066)
  • R1-6 initialIndex degrades to 0 when the active style is not listed — still standing; already reported (comment 3914106079)
  • R1-23 unawaited dialog open can re-open a dismissed dialog — still standing; already reported (comment 3914106089)
  • R1-60 pattern: tests assert outcomes without discriminating the mechanism — still standing; already reported (comment 3914106093)
  • R1-48 size bound 41x the house bound, enforced on stat.size while the read is unbounded — still standing; already reported (comment 3914106104)
  • R1-7 frontmatter fence grammar too strict and too loose — still standing; already reported (comment 3914106112)
  • R1-58 HTML comments survive into prompt and derived description — still standing; already reported (comment 3914106124)
  • R1-20 a file shadowing a built-in inherits keep-coding-instructions:false — still standing; already reported (comment 3914106132)
  • R1-22 keep-coding-instructions coerced by exact two-value comparison — still standing; already reported (comment 3914106141)
  • R1-49 every *.md entry decoded as UTF-8 with no text sniff — still standing; already reported (comment 3914106150)

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

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

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour.

Not reviewed: build-and-test — test-efficacy probe inconclusive (the probe scratch tree has no built dist, so no green baseline); full packages/cli and packages/core suites timed out at the per-run budget on this saturated runner; every test file this PR adds or changes was run individually and passes.

Test Plan (not a blocker): src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 7 more.

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

  • packages/cli/src/ui/commands/output-style-command.test.ts:105 — [probe] the listing-message assertion matches only the i18n key — vacuous under the t mock (round-2 code-age rule: anchored on code unchanged since round 1, which read it and d…
  • packages/cli/src/config/config.ts:1374 — [probe] the startup unknown-style warning's catalog names have no test — reverting to built-ins ships green (round-2 code-age rule)
  • packages/core/src/core/output-style-files.test.ts:91 — [probe] the fallback-name validation path has no test pin — a bare default.md mutant escapes the suite green (round-2 code-age rule)
  • packages/core/src/core/output-style-files.ts:119 — [probe] deriveDescription mixes and ~~~ fence toggles — the description can come from inside a code block (round-2 code-age rule)

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

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

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

未审查:reverse audit — did not converge within the reverse-audit round cap of 5。

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

未审查:build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour。

未审查:build-and-test — test-efficacy probe inconclusive (the probe scratch tree has no built dist, so no green baseline); full packages/cli and packages/core suites timed out at the per-run budget on this saturated runner; every test file this PR adds or changes was run individually and passes。

Test Plan(非阻断):src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 7 more。

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

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff,除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上;并会一直如此,直到某一轮的标记重新带上锚点,或落地的嫁接能被运行该轮的评审使用。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

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

Comment thread packages/cli/src/ui/components/DialogManager.tsx
Comment thread packages/cli/src/ui/hooks/use-output-style-command.test.ts
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts
Comment thread docs/design/2026-09-02-output-style-files.md Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts Outdated
@qqqys

qqqys commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover stop

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

Copy link
Copy Markdown
Collaborator

👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply autofix/takeover (or comment @qwen-code /takeover) to re-engage.

中文说明

👋 已释放:autofix 循环不再介入此 PR(在飞的一轮如有,将完成其有界工作)。重新打上 autofix/takeover 标签(或评论 @qwen-code /takeover)即可再次接管。

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.
…t 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.
`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.
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.
@qqqys

qqqys commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

All five Criticals addressed — 2e3b878d7

Takeover is off; this round is manual. Four commits, each with the mutation flip the review asked for.

R1-19 symlink/hard-link exfiltration (599441cd2) — confirmed. A project file is now read with lstat, so a link is not a regular file and never reaches the read; 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 (not ~/.qwen, or that setup would break). Both refuse nlink > 1 and both realpath-confine, which is what catches a symlinked ancestor. Mirrors readLoopTaskFile as you suggested.

One correction to the suggested test set: the four cases you listed do not distinguish lstat from confinement. Dropping the symlink refusal left all of them green, because the project symlink in that case pointed outside the root and confinement caught it. The case that separates them is a project symlink whose target is in-workspacenotes.md -> .env, which a repo can commit to read a developer's own secrets and which confinement waves through. That case is in, and flipping lstat to stat turns it red. The explicit isSymbolicLink() branch changes no outcome (an lstat-ed symlink already fails isFile()); it is kept only so a refusal is logged instead of silent, and the comment says so.

R1-2 un-sanitized description (599441cd2) — both sources now go through stripAnsiAndControl plus a \p{Cf} strip, collapse to one line, and share MAX_DERIVED_DESCRIPTION_LENGTH. Removing the sanitize turns two cases red.

R1-3 headless Learning rule (cd8d0435b) — keyed on source === 'built-in' && name === 'Learning'. Removing the source clause turns the new case red; the built-in is still dropped.

R1-54 vacuous test + re-open defect (994a82800) — the deleted openOutputStyleDialog() is restored with the siblings' waitFor, and the hook takes an open generation retired both by a newer open and by a selection. A new case dismisses an open and then resolves its load, asserting the dialog stays closed; removing the guard turns it red.

R1-1 OpenTUI picker (2e3b878d7) — the dialog loads through loadSessionOutputStyles (carrying the ink trust gate), labels non-built-ins with their source, and re-derives the pre-selection when the catalog arrives, since useState runs its initialiser once. The test stubs the catalog load so it never reads the developer's own ~/.qwen/output-styles; reverting the list to BUILT_IN_OUTPUT_STYLES turns it red.

Verificationoutput-style-files + output-styles 51 passed; use-output-style-command, dialogs-modes, output-style-command, config, opentui-dialog-mount, commands-dispatch 503 passed; tsc --noEmit clean for packages/core; eslint and prettier clean.

The Suggestions (R1-6/7/8/20-23/48/49/53/58/60-62, R2-1..4) are not in this round. Happy to take them next if you would rather they land here.

中文说明

takeover 已关,本轮手动处理。四个 commit,每条都做了评审要求的变异验证。

  • R1-19 软/硬链接外泄599441cd2):project 文件改用 lstat,链接不是普通文件,永远走不到读取;user 文件允许是链接(~/.qwen/output-styles/x.md -> ~/dotfiles/x.md 是常见配置),但其规范路径必须落在 home 根内(不是 ~/.qwen,否则这个配置就废了)。两者都拒绝 nlink > 1,都做 realpath 收敛以捕获软链接祖先。按你的建议对齐 readLoopTaskFile

    对建议测试集有一处修正:你列的四个用例区分不出 lstat 与路径收敛。把软链接拒绝去掉后四个仍全绿,因为那个 project 软链接指向根,被收敛拦下了。真正能区分的是目标在工作区内的 project 软链接——notes.md -> .env,仓库可以提交它来读取开发者自己的密钥,而路径收敛会放行。该用例已加入,把 lstat 改成 stat 即变红。显式的 isSymbolicLink() 分支不改变任何结果(lstat 到的软链接本就不是 isFile()),保留只为让拒绝有日志而非静默消失,注释里写明了这一点。

  • R1-2 描述未净化599441cd2):两个来源都过 stripAnsiAndControl\p{Cf} 剥离,压成一行,共用 MAX_DERIVED_DESCRIPTION_LENGTH。去掉净化会让两个用例变红。

  • R1-3 headless Learning 规则cd8d0435b):改为以 source === 'built-in' && name === 'Learning' 为键。去掉 source 条件会让新用例变红,内置 Learning 仍被丢弃。

  • R1-54 空断言测试 + 重开缺陷994a82800):被删掉的 openOutputStyleDialog() 已按同类测试的 waitFor 形式恢复;hook 增加 open 代次,新的 open 与选择都会作废旧代次。新用例先关闭再让加载完成,断言对话框保持关闭;去掉守卫即变红。

  • R1-1 OpenTUI 选择器2e3b878d7):改为经 loadSessionOutputStyles 加载(沿用 ink 的信任门),非内置项标注来源,并在目录到达后重新推导预选项(useState 初始化器只跑一次)。测试打桩了目录加载,不会读开发者本机的 ~/.qwen/output-styles;把列表改回 BUILT_IN_OUTPUT_STYLES 即变红。

验证:core 51 绿;CLI 六个套件 503 绿;packages/coretsc --noEmit 干净;eslint 与 prettier 干净。

Suggestion 类这轮未处理,需要在本 PR 一并解决的话我再补一轮。

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

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

  • R2-1 design doc picker bullet backwards (clear error vs silent snapshot apply) — still standing; already reported (comment 3921888866)
  • R2-2 file-name-derived style name skips trimming — still standing; already reported (comment 3921888873)
  • R2-3 non-string YAML name String()-coerced without type check — still standing; already reported (comment 3921888888)
  • R2-4 home-directory guard compares os.homedir() while the user level resolves through QWEN_HOME — still standing; already reported (comment 3921888898)
  • R1-60 occurrence 12 — vacuous i18n-key assertion at output-style-command.test.ts:105 — already reported (comment 3914106093)
  • R1-60 occurrence 13 — startup unknown-style warning list at config.ts:1374 has no test pin — already reported (comment 3914106093)
  • R1-62 occurrence — settings.md:808 --output-style row still a closed built-in enumeration — already reported (comment 3914106056)
  • R1-62 occurrence — settings.md:83 "custom style file's name" wording — already reported (comment 3914106056)
  • R1-61 occurrence 6 — E2E plan scenario 8 cannot exercise the trust gate — already reported (comment 3914106020)
  • R1-6 ink picker initialIndex fallback re-discovered at OutputStyleDialog.tsx:49-51 — already reported (comment 3914106079)

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

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour.

Not reviewed: build-and-test — test-efficacy probe could not validate its positive control (the probe scratch tree tripped the repo's vitest globalSetup dist-guard, harnessValidated null); full packages/cli and packages/core suites timed out at their per-run deadlines on this runner; every test file this PR adds or changes was run individually and passes (494/494).

Not explored to full depth (tool budget reached): chunk 3: running the new tests in packages/cli/src/config/config.test.ts and packages/cli/src/ui/commands/output-style-command.test.ts — npm run build (the documen….

Not reviewed: reverse audit — stopped before round 5 by the review time budget.

Test Plan (not a blocker): src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more.

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

  • packages/core/src/core/output-style-files.ts:150 — [probe] deriveDescription emphasis strip corrupts identifiers (MAX_RETRY_COUNT → MAXRETRYCOUNT) in picker descriptions (round-3 code-age rule: anchored on code unchanged since round 2, whic…
  • packages/core/src/core/output-style-files.ts:290 — [probe] getUserOutputStylesRoot() branches (QWEN_HOME relocation vs home-root dotfiles) have no discriminating test; both regression mutants ship green (round-3 code-age rule: anchored on c…
  • packages/core/src/core/output-style-files.ts:323 — [probe] includeProject home-dir skip unguarded for empty os.homedir(); path.resolve('') is cwd, so a trusted workspace's project styles silently never load (round-3 code-age rule: anchored …

Convergence: round 3 posted 6 inline comment(s), 6 of them reported for the first time; the previous round posted 9 (4 new). Findings keep coming back to the same files: packages/cli/src/ui/hooks/use-output-style-command.test.ts (findings in round 1; 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. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

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

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

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

未审查:build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour。

未审查:build-and-test — test-efficacy probe could not validate its positive control (the probe scratch tree tripped the repo's vitest globalSetup dist-guard, harnessValidated null); full packages/cli and packages/core suites timed out at their per-run deadlines on this runner; every test file this PR adds or changes was run individually and passes (494/494)。

未探索到全部深度(达到工具调用预算):chunk 3:running the new tests in packages/cli/src/config/config.test.ts and packages/cli/src/ui/commands/output-style-command.test.ts — npm run build (the documen…

未审查:反向审计——评审时间预算不足,未能开始第 5 轮。

Test Plan(非阻断):src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more。

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

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

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

Comment thread packages/cli/src/ui/opentui/dialogs-modes.tsx Outdated
Comment thread packages/cli/src/ui/hooks/use-output-style-command.ts Outdated
Comment thread packages/cli/src/ui/hooks/use-output-style-command.ts Outdated
Comment thread packages/cli/src/ui/opentui/dialogs-modes.tsx
Comment thread packages/cli/src/ui/hooks/use-output-style-command.test.ts
Comment thread packages/cli/src/ui/opentui/dialogs-modes.tsx Outdated

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

  • R2-2 fallback style name skips trimming (incl. ' default.md' reserved-name bypass variant) — already reported (comment 3921888873)
  • R1-62 occurrence — settings.md:83 'custom style file's name' wording — already reported (comment 3914106056)
  • R1-62 occurrence — settings.md:808 --output-style row still a closed built-in enumeration — already reported (comment 3914106056)

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

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only.

Not reviewed: build-and-test — test-efficacy probe inconclusive (harnessValidated null: the probe scratch tree tripped the repo's vitest globalSetup dist-guard); mutant coverage neither validated nor refuted.

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

Test Plan (not a blocker): src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more.

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

  • packages/cli/src/ui/commands/output-style-command.test.ts:105 — [probe] the listing assertion matches only the i18n key under the identity t mock; OUTPUT_STYLE_LIST mutant ships green (round-4 code-age rule; round-2 deferral re-confirmed)
  • packages/core/src/core/output-style-files.ts:290 — [probe] getUserOutputStylesRoot's no-QWEN_HOME branch is never exercised; unconditional-Storage.getGlobalQwenDir() mutant ships green (round-4 code-age rule; round-3 deferral re-confirmed)
  • packages/core/src/core/output-style-files.ts:323 — [probe] includeProject home-skip mis-fires under QWEN_HOME relocation, silently dropping home-rooted project styles (round-4 code-age rule)
  • packages/core/src/core/output-style-files.test.ts:30 — [probe] keep-coding-instructions: "true" quoted-string branch unwitnessed; strict-boolean mutant ships green (round-4 code-age rule)
  • packages/core/src/core/output-style-files.test.ts:123 — [probe] \p{Cf} class of the name-rejection regex unwitnessed; dropping it accepts U+202E names into the picker and prompt heading (round-4 code-age rule)
  • packages/core/src/core/output-style-files.test.ts:110 — [probe] derived-description >120 truncation branch unwitnessed; cap-before-strip ordering slices escape-padded lines to a junk prefix (round-4 code-age rule)
  • packages/core/src/core/output-styles.ts:168 — [review] OutputStyleSource contract comment still claims only 'built-in' is populated, which this PR falsifies (round-4 code-age rule)
  • packages/core/src/core/output-style-files.test.ts:281 — [probe] QWEN_HOME confinement root rejects the dotfiles symlink setup the docstring endorses; neither branch's production-root semantics is pinned (round-4 code-age rule)
  • packages/core/src/core/output-style-files.test.ts:189 — [probe] hard caps tested only from the rejection side; off-by-one mutants ship green and refuse files at the documented maximum (round-4 code-age rule)
  • packages/core/src/core/output-style-files.test.ts:309 — [probe] user-over-built-in precedence tier unpinned; winners-iteration reorder mutant ships green (round-4 code-age rule)
  • packages/core/src/core/output-styles.test.ts:171 — [probe] headless keep-rule pinned only for source 'user'; source !== 'user' mutant ships green and drops project Learning styles (round-4 code-age rule)

Convergence: round 4 posted 5 inline comment(s), 5 of them reported for the first time; the previous round posted 6 (6 new). Findings keep coming back to the same files: packages/cli/src/ui/opentui/dialogs-modes.tsx (findings in rounds 1, 3; 3 more now); packages/cli/src/ui/hooks/use-output-style-command.ts (findings in round 3; 1 more now). 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. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

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

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

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

未审查(原文为英文):build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only.

未审查(原文为英文):build-and-test — test-efficacy probe inconclusive (harnessValidated null: the probe scratch tree tripped the repo's vitest globalSetup dist-guard); mutant coverage neither validated nor refuted.

未审查:反向审计——在 5 轮的反审轮数上限内未收敛。

Test Plan(非阻断):src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more。

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

收敛情况:第 4 轮发布了 5 条行内评论,其中 5 条是首次提出;上一轮发布了 6 条(其中 6 条首次提出)。发现反复回到同一批文件:packages/cli/src/ui/opentui/dialogs-modes.tsx(第 1、3 轮已出过发现,本轮又有 3 条);packages/cli/src/ui/hooks/use-output-style-command.ts(第 3 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

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

Comment thread packages/cli/src/ui/opentui/dialogs-modes.tsx Outdated
Comment thread packages/cli/src/ui/opentui/dialogs-modes.tsx Outdated
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/opentui/dialogs-modes.tsx
wenshao and others added 5 commits September 4, 2026 09:19
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.
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.
…s 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.
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.

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

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

  • the no-QWEN_HOME branch of getUserOutputStylesRoot() is never exercised by any test — already recorded in the round-3 deferral paragraph and re-confirmed in the round-4 deferral paragraph

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

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour.

Not reviewed: build-and-test — packages/cli suite timed out at its per-run deadline and packages/webui did not run (whole-call budget on this runner); every test file this PR adds or changes was run individually and passes.

Not reviewed: build-and-test — test-efficacy probe could not validate its positive control (harnessValidated null: the probe scratch tree tripped the repo's vitest globalSetup dist-guard); mutant coverage neither validated nor refuted.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": trace the daemon trust-policy monitor's onSnapshot follow-through (whether revocation drains/restarts workspace sessions, which would end the embedded-style exp….

Not reviewed: reverse audit — stopped before round 5 by the review time budget.

Test Plan (not a blocker): src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more.

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

  • packages/core/src/core/output-style-files.ts:236 — [probe] deriveDescription returns a bare '---' thematic break as the picker description (round-5 code-age rule: anchored on code unchanged since round 4, which read it and did not flag it)
  • packages/cli/src/ui/commands/output-style-command.ts:97 (+2 locations) — [probe] pattern: unknown-name error interpolations unpinned at output-style-command.ts:97 and use-output-style-command.ts:137; mutations ship green (round-5 code-age r…
  • packages/core/src/core/output-style-files.ts:146 — [probe] unknown-frontmatter-key warning is unreachable; an unknown key silently demotes the whole frontmatter to prose (round-5 code-age rule: anchored on code unchanged since round 4)
  • packages/core/src/core/output-style-files.test.ts:251 — [probe] sanitization tests assert absence only; an over-stripping sanitizer ships green (round-5 code-age rule: anchored on code unchanged since round 4)

Convergence: round 5 posted 9 inline comment(s), 9 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: packages/cli/src/ui/opentui/dialogs-modes.tsx (findings in rounds 1, 4; 1 more now); packages/cli/src/ui/hooks/use-output-style-command.ts (findings in round 4; 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. (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.

未审查(原文为英文):build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only, and this diff touches path, homedir, symlink and file-encoding behaviour.

未审查(原文为英文):build-and-test — packages/cli suite timed out at its per-run deadline and packages/webui did not run (whole-call budget on this runner); every test file this PR adds or changes was run individually and passes.

未审查(原文为英文):build-and-test — test-efficacy probe could not validate its positive control (harnessValidated null: the probe scratch tree tripped the repo's vitest globalSetup dist-guard); mutant coverage neither validated nor refuted.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)"trace the daemon trust-policy monitor's onSnapshot follow-through (whether revocation drains/restarts workspace sessions, which would end the embedded-style exp…

未审查:反向审计——评审时间预算不足,未能开始第 5 轮。

Test Plan(非阻断):src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 9 more。

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

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

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

Comment thread packages/cli/src/ui/commands/output-style-utils.ts Outdated
Comment thread packages/core/src/core/output-style-files.ts
Comment thread packages/core/src/core/output-style-files.ts
Comment thread packages/cli/src/ui/hooks/use-output-style-command.ts
Comment thread packages/cli/src/ui/opentui/dialogs-modes.tsx
Comment thread packages/core/src/core/output-style-files.ts
Comment thread packages/core/src/core/output-style-files.ts
Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/vscode-ide-companion/schemas/settings.schema.json

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

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

  • unknown-name error interpolation at output-style-command.ts:96 — already recorded in the round-5 deferral paragraph

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

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only.

Test Plan (not a blocker): src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 7 more.

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

  • packages/cli/src/ui/commands/output-style-utils.ts:126 — [probe] D6-1 new untrusted-workspace branch's source === 'project' conjunct is unpinned; deleting it ships green
  • packages/core/src/core/output-style-files.ts:380 — [review] R5-2 UTF-16 guard relies solely on isBinary's NUL-byte sniff; single-line UTF-16LE without ASCII/whitespace evades it
  • packages/core/src/core/output-style-files.ts:458 — [review] R5-3 duplicate-load guard compares raw path.resolve strings — no case folding, no symlink resolution
  • packages/cli/src/ui/hooks/use-output-style-command.ts:115 — [review] R5-4 append guard checks name membership only; a same-named different definition silently replaces the live one
  • packages/cli/src/ui/opentui/dialogs-modes.tsx:304 — [review] R5-5 append-live-style branch cannot distinguish 'file no longer parses' from 'excluded by trust policy'
  • packages/core/src/core/output-style-files.ts:55 — [review] R5-6 frontmatter fence closer anchored to end-of-line only; a value line ending in ' ---' closes the block mid-frontmatter
  • packages/core/src/core/output-style-files.ts:79 — [review] R5-7 key-line grammar narrower than YAML; quoted or spaced-colon keys demote valid frontmatter to prose
  • packages/cli/src/config/settingsSchema.ts:446 — [review] R5-8 closing sentence 'Only the built-ins are available in --bare and --safe-mode' misdescribes the setting
  • packages/vscode-ide-companion/schemas/settings.schema.json:74 — [review] R5-9 closing sentence 'Only the built-ins are available in --bare and --safe-mode' misdescribes the setting
中文说明

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

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

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

未审查(原文为英文):build-and-test — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) were skipped in CI; the unit suite ran on Linux only.

Test Plan(非阻断):src/core/output-style-files.test.tsno such file or directory; src/core/output-styles.test.tsno such file or directory; 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; and 7 more。

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

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

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

  • History: five earlier CHANGES_REQUESTED rounds all predate the current head; the fixes they forced (session-scoped resolution, prompt-safe file reading, steady picker, inactive-project explanation) are in the five most recent commits, all 48 review threads are resolved with the author's replies recorded, and the last formal pass at this head (ci-bot, 11:53) posted no new Critical — its one open item is the previously recorded S-level unknown-name interpolation note.
  • Independent pass on the new loader found no Criticals. The file surface is bounded the way prompt-injected files should be: 25 KB cap matched to the house bound with the +1-byte read, control-character stripping, reserved-name and length checks, and — for untrusted-by-definition project files — lstat rejecting the final-component symlink, nlink > 1 rejecting hardlinks, and a realpath containment check against the workspace root before the file is read; the caller only passes projectRoot when the folder is trusted, and --bare/--safe-mode collapse to built-ins. The picker resolving against the list it displayed, and invalid files skipping with a log rather than killing the catalog, are both pinned by the new suites.
  • Author evidence is execution-grade (real npm run dev picker capture, haiku applied end to end, focused 41+247+28 tests green), and the one remaining pre-existing tsc error in the OpenTUI commands registry is honestly disclosed as out of scope.
  • CI facts: every completed check on this head passes — 19/19, zero failures, nothing pending. Per the channel convention the call is on the review itself, and here the lanes agree.
  • Housekeeping note: the page may still read CHANGES_REQUESTED from the bot's round-5 ticket (filed at 71c71968, superseded by its own re-review of this head); a triage re-run lifts it. That ticket does not reflect any unresolved finding at bb936adf.

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

Verified at head bb936ad against the three open Criticals from the last round — all fixed and test-pinned:

  • R1-1 (fix-induced picker reload): the catalog-load effect now keys on [config] only, with onClose/notify mirrored into refs synced per render; pinned by 'keeps the navigated row when the shell re-renders with new callbacks' (load runs once, selection survives).
  • R4-1 (active style clamped to index 0): a live style missing from the re-read catalog is now appended to the list (case-insensitive identity) instead of snapping to default; pinned by 'lists the active style the reloaded catalog no longer carries' and the case-duplicate guard, with the ink twin fixed in the hook too.
  • R5-1 (untrusted-workspace project style misattributed): applyOutputStyleSelection now has a dedicated project source + untrusted workspace explanation ahead of the headless-Learning branch, with locale entries and a test asserting the trust reason (and the absence of the headless message).

Test (ubuntu), Lint & Static and Integration Tests (no-AK) all green on this head; no unresolved threads.

Non-blocking, carried on the record: R5-2 (UTF-16 files without NUL in the sample window evade the binary sniff), R5-3 (home/project dir comparison is lexical — no case folding/realpath), R5-4/R5-5 (append guard keys on name membership; a trust-dropped project style can still be listed/pre-selected with the ● marker), R5-6/R5-7 (frontmatter fence anchored to end-of-line only; key grammar narrower than YAML), R5-8 (the new '--bare/--safe-mode' sentence misdescribes the setting — the setting is ignored entirely in those modes). Also minor: the two new scope-mismatch messages from 71c7196 ('…is a personal/project style, so it will not resolve…') have no locale entries in any of the nine locale files, so they will render as raw keys.

@qqqys
qqqys added this pull request to the merge queue Sep 4, 2026
Merged via the queue into QwenLM:main with commit d084730 Sep 4, 2026
365 of 378 checks passed
@chiga0

chiga0 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Post-merge review of head bb936adf (merged during review; no approval event). Deep-tier pass over the loader's security surface and all three CLI consumers, executed locally on linux / node v24.20.0.

Verified in the code at this head

  • File surface is bounded the way a prompt-bearing file should be: 25 kB cap enforced on the bytes actually read (cap+1), binary/UTF-16 sniff before decode, HTML comments stripped from the body, name validation (reserved default, control/format characters refused, length cap), description sanitized for picker rendering (ANSI, C0/C1, \p{Cf}, one line), and a frontmatter fence that discriminates real frontmatter from decorative --- prose.
  • Link handling matches the trust split: project files refuse symlinks at the final component (lstat), refuse hard links (nlink > 1), and realpath-confine to the workspace root — covering the checked-in .qwen -> /outside ancestor case; user files may be links but stay confined to the home/QWEN_HOME root.
  • Trust gating on both axes: loadCliConfig collapses to built-ins under --bare/--safe-mode and passes projectRoot only when the folder is trusted; resolveMainSessionOutputStyle (and the prompts consumer) re-check the live verdict, so a project style stops applying the moment trust flips mid-session.
  • Catalog semantics: case-insensitive dedup with project > user > built-in precedence, picker order built-in → user → project; the picker and /output-style <name> re-read the catalog per use, resolve the selection against the list actually displayed, carry a generation guard for the async open/close race, and offer a live-but-missing active style rather than snapping the selection to default.
  • Headless Learning drop keys on source === 'built-in' (a custom style named Learning survives headless), and arena peers inherit a style only when it keeps the coding instructions they're judged on.

Executed: full workspace build clean (0 TS errors) · core suites (output-style-files, output-styles, prompts, ArenaManager) 258/258 · cli suites (output-style-command, hook, both dialogs, dialogs-modes, config) 467/467. Two mutation probes, both killed: allowing project symlinks through the loader, and inverting the project/user precedence.

Cross-check: the two existing approvals at this head match my read — the last round's three Criticals are fixed and test-pinned, and the carried non-blocking items (UTF-16-without-NUL sniff window, lexical dir comparison, fence/grammar narrowness, the --bare doc sentence, two missing locale entries) are recorded follow-ups, none affecting what merged. One PR-body nit for the record: the body says "over 1 MiB" where code and shipped docs both say 25 kB — the docs are right. Not covered here: live picker E2E on a real terminal, Windows/macOS behaviour (linux only).

No blockers found in what merged.

Reviewed with AI assistance.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.1.

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.

6 participants