feat(review): emit Step 3A's fan-out as a workflow instead of a roster - #8846
feat(review): emit Step 3A's fan-out as a workflow instead of a roster#8846qqqys wants to merge 5 commits into
Conversation
`agent-prompt --roster` and the new `emit-workflow` build the same prompts, from the same plan, through the same function. What differs is who launches them. `--roster` prints ~13 blocks and asks the orchestrator to copy each one into an agent call, in a single response, without editing any of them. Those are three conventions, and this skill's gate list exists because each has been broken: a roster that got shortened, prompts that arrived paraphrased, agents launched with no diff path at all. Every one of those gates detects the failure after the fact. `emit-workflow` writes the same blocks into an args file a fixed script reads, so the orchestrator makes one tool call naming a path. The fan-out's width becomes a `parallel()` over an array, and each prompt becomes a value passed by code — neither is something a caller can shorten or reword. What this deliberately does NOT change: the briefs, the prompts, the roster, and how findings come back. The agents are the same agents reading the same briefs. That is the point — this exists to be A/B'd against the hand-launched path, and a change that also moved the finding format would be comparing two of everything. Three things carry the invariant rather than restating it: - `buildLaunch` is exported and reused rather than reimplemented. Its own comment already says one body serves every caller because a drift would read as a rewritten launch on a run that did everything right; a second builder would have made byte-parity something a test asserts instead of something the code cannot break. - The roster comes from `requiredAgents(plan)` — the same list `check-coverage` holds the run to — and the same key-mismatch guard `--roster` makes is made here, so a prompt that could never be matched to its requirement is refused rather than handed out. - The script is a fixed constant, not generated source. Generated source can only be checked by parsing it; a constant can be executed against a stub dispatch and asserted on. It is also where "the orchestrator must not author the fan-out" stops being a convention and becomes a fact about the binary. The script collects `parallel()`'s null elements into `missingRoles` by name. A dead dispatch is otherwise indistinguishable from an agent that found nothing, and an agent silently missing from the fan-out is the one regression this path must not introduce. Two refusals rather than fallbacks: a territory fan-out (3B) has a per-chunk contract this script does not express, and a worktree review needs every agent pinned to the PR worktree, which the workflow runtime cannot yet express. Either would produce a run that looks complete and reviewed the wrong thing. Not wired into SKILL.md: this build emits, nothing routes through it yet, so review behaviour is unchanged. Part of QwenLM#8769. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@qqqys Thanks for the PR — the direction is clear (it is exactly the emit-workflow step designed in #8769, and "nothing routes through it yet" is sensible scoping), but the body doesn't follow the PR template, so I have to stop here before code review.
What's missing:
## Reviewer Test Plan— the big one. The## Testssection describes the suite well, but there is no How to verify for a reviewer: what command to run, what plan input to feedqwen review emit-workflow, what output to expect — plus the Tested on OS matrix. The template opens with "Maintainers prioritize PRs with a clear reviewer test plan."## Risk & Scope— most of this is already in your "What it deliberately does not change" and "Two refusals, not fallbacks" sections; it just needs to sit under the template heading (main risk/tradeoff, what's not validated, breaking changes).## Linked Issues—Part of #8769is only mentioned in prose at the end.## What/## Why→ the template's## What this PR does/## Why it's needed(the content itself can stay as-is).
Your recent PRs (#8694, #8735) follow the template exactly, so this reads as a formatting slip rather than a disagreement with it. Restructuring the existing content under the template headings — and adding the reviewer verification plan — should be enough to move this into code review.
中文说明
@qqqys 感谢这个 PR——方向很明确（正是 #8769 设计中的 emit-workflow 那一步，"目前没有任何东西路由到它"的范围控制也很合理），但 PR 正文没有遵循 PR 模板，所以必须先停在模板检查这一步，无法进入代码审查。
缺失的部分：
## Reviewer Test Plan——最关键的一项。## Tests一节对测试套件描述得很好，但缺少给 reviewer 的 How to verify：运行什么命令、给qwen review emit-workflow喂什么 plan 输入、期望看到什么输出，以及 Tested on 操作系统矩阵。模板开头写明"维护者优先处理有清晰评审验证计划的 PR"。## Risk & Scope——内容其实已经在 "What it deliberately does not change" 和 "Two refusals, not fallbacks" 两节里了，只需要挪到模板标题下（主要风险/权衡、未验证内容、破坏性变更）。## Linked Issues——Part of #8769目前只是正文结尾的一句提及。## What/## Why→ 模板的## What this PR does/## Why it's needed（内容可以原样保留）。
你最近的 PR（#8694、#8735）都严格遵循了模板，所以这次看起来只是格式上的疏忽。把现有内容重新组织到模板标题下、并补上给 reviewer 的验证计划，就可以进入代码审查了。
— Qwen Code · qwen3.8-max
|
已修复 验证证据：
|
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
qwen-code-dev-bot
added
the
autofix/takeover
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8846 (QwenLM/qwen-code) adds qwen review emit-workfl...: did not execute the new vitest suites ( emit-workflow.test.ts , workflow-script.test.ts ); findings here are static contract mismatches that unit tests of the …`.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8846 (QwenLM/qwen-code) adds qwen review emit-workfl...：did not execute the new vitest suites ( emit-workflow.test.ts , workflow-script.test.ts ); findings here are static contract mismatches that unit tests of the …`。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const { key, prompt } = buildLaunch( | ||
| report, | ||
| planPath, | ||
| { role: req.role, file: req.file }, | ||
| rules, | ||
| ); |
There was a problem hiding this comment.
[Critical] R1-1: buildWorkflowArgs builds every launch prompt via buildLaunch but never calls recordPrompt, while the sibling --roster path records every prompt it hands out (runRoster, agent-prompt.ts:1764). The delivery gate reads only those records: coverageFromTranscripts builds its built map from readRecordedPrompts (lib/coverage.ts:384), which reads only the .txt files recordPrompt writes — the *.brief.md files writeBrief creates are explicitly not what the gate compares.
Failure scenario: anyone who follows this command's own stdout ("make one Workflow call with the paths below") and dispatches the emitted workflow gets a fully correct run that check-coverage then fails wholesale — every roster role classifies as briefless, and the gate exits 3 with "required briefs never reached their agents", prescribing the wrong repair (agent-prompt --roster). Probe-verified at this commit: the handler produces 13 prompts and 0 records; adding the line below flips the probe to 13/13 records with the PR's tests still green.
Fix (mirror runRoster):
recordPrompt(planPath, key, prompt); // after the key-mismatch guard; import from './lib/prompt-record.js'中文说明
buildWorkflowArgs 通过 buildLaunch 为每个 agent 构建启动 prompt，但从未调用 recordPrompt；而姊妹路径 --roster（runRoster，agent-prompt.ts:1764）会为它发出的每个 prompt 做记录。交付门禁只读这些记录：coverageFromTranscripts 的 built 表来自 readRecordedPrompts（lib/coverage.ts:384），只认 recordPrompt 写出的 .txt 文件——writeBrief 写出的 *.brief.md 明确不是门禁比对的对象。
失败场景：任何人按本命令自己 stdout 的指引（"make one Workflow call with the paths below"）分发这个 workflow，都会得到一次完全正确、却被 check-coverage 整体判失败的运行——每个 roster 角色都被归为 briefless，门禁以 "required briefs never reached their agents" 退出（exit 3），并给出错误的补救指引（agent-prompt --roster）。已在本提交上用探针验证：handler 产出 13 个 prompt、0 条记录；加上下面这一行后探针变为 13/13，且 PR 自带测试仍然全绿。
修复（对齐 runRoster）：在 key 不一致守卫之后加 recordPrompt(planPath, key, prompt);（从 ./lib/prompt-record.js 导入）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| writeStdoutLine( | ||
| `${payload.agents.length} agents required. The fan-out is a workflow — ` + | ||
| 'make one Workflow call with the paths below and do not build agent ' + | ||
| 'calls by hand for this step.', | ||
| ); |
There was a problem hiding this comment.
[Critical] R1-2: The printed handoff contract — "make one Workflow call with the paths below" — cannot be honored against the Workflow tool in this build, for three code-verified reasons:
- The tool's
argsparameter accepts inline JSON only ("Pass actual JSON, not a stringified value" —WORKFLOW_PARAM_SCHEMAin packages/core/src/tools/workflow/workflow.ts); there is noargsPathparameter. Passing the printedargs:path string binds that string to the sandboxargsglobal, soargs.agentsisundefinedand the script dies inside the vm with a bareTypeErroratagents.length. scriptPathis resolved throughresolveSavedWorkflowScript→readWorkflowFileSecurely, which refuses any path outside.qwen/workflows/or~/.qwen/workflows/— the emitted--outdir is arbitrary.workflowsEnableddefaults tofalse(config.ts), so a default session has no Workflow tool to call at all.
Failure scenario: an orchestrator runs emit-workflow and mechanically follows its printed output (the command's stated purpose) — and hits one of the three walls above. The only executable route today is reading args.json and inlining ~13 full launch prompts into the tool call, which re-introduces exactly the relay burden this command's header says it exists to remove ("the prompts are in the args file, which no one is asked to read, retype, or relay").
Suggested fix: land the interface the artifact assumes before routing — an argsPath parameter on WorkflowTool read at execution time like scriptPath, plus a trusted source for review-emitted scripts outside the saved-workflow dirs, gated on isWorkflowsEnabled(). Until then, the printed text should not describe a call the tool cannot honor.
中文说明
打印出来的交接契约——"make one Workflow call with the paths below"——在当前构建的 Workflow 工具上是无法执行的，原因有三（均已在代码中核实）：
- 工具的
args参数只接受内联 JSON（"Pass actual JSON, not a stringified value"——packages/core/src/tools/workflow/workflow.ts 中的WORKFLOW_PARAM_SCHEMA），不存在argsPath参数。把打印出的args:路径字符串传进去，会把该字符串绑定到沙箱的args全局变量，于是args.agents为undefined，脚本在 vm 内部以裸TypeError（agents.length）终止。 scriptPath经过resolveSavedWorkflowScript→readWorkflowFileSecurely解析，会拒绝.qwen/workflows/或~/.qwen/workflows/之外的任何路径——而--out目录是任意的。workflowsEnabled默认为false（config.ts），默认会话里根本没有 Workflow 工具可调。
失败场景：orchestrator 运行 emit-workflow 并机械地照打印输出执行（这正是该命令的既定用途）——然后撞上上述三堵墙之一。当前唯一可执行的路径是读取 args.json 并把约 13 条完整启动 prompt 内联进工具调用，这恰好重新引入了本命令头注释声称要消除的中转负担（"prompts 都在 args 文件里，没有人需要阅读、转述或转发它们"）。
建议修复：在接线（routing）之前先落地该产物所假定的接口——给 WorkflowTool 增加一个在执行期读取的 argsPath 参数（与 scriptPath 相同），并为 review 产出的脚本提供 saved-workflow 目录之外的可信来源，且以 isWorkflowsEnabled() 作门禁。在此之前，打印文本不应描述一个工具无法履行的调用。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Verified against this commit — all three walls are real:
WORKFLOW_PARAM_SCHEMA(packages/core/src/tools/workflow/workflow.ts) has noargsPathparameter;argsis an inline-only structured value.scriptPathresolves throughresolveSavedWorkflowScript→readWorkflowFileSecurely(packages/core/src/agents/runtime/workflow-saved.ts), which refuses any path outside the saved-workflow directories — the arbitrary--outdir included.workflowsEnableddefaults tofalse(packages/core/src/config/config.ts), and the Workflow tool is only registered when it is enabled.
So the finding stands: the printed handoff contract cannot be honored by the Workflow tool in this build. What I am NOT doing is settling the fix direction unilaterally, because it is a scope/sequencing call, not a code call:
- (a) Keep this PR emit-only (its declared scoping: "nothing routes through it yet") and land the Workflow-tool interface the artifact assumes — an
argsPathparameter plus a trusted source for review-emitted scripts, gated onisWorkflowsEnabled()— as the next Proposal: rebuild /review Step 3–5 orchestration on the workflow engine #8769 step, before routing. This keeps the core-package interface change in its own dedicated change. - (b) Widen this PR to land that interface now. That is a
packages/coretool-schema and script-loading-policy change, a scope expansion beyond the PR's stated emit-only intent. - (c) Soften the stdout text so it stops describing a call the tool cannot yet honor, at the cost of rewriting the handoff contract Proposal: rebuild /review Step 3–5 orchestration on the workflow engine #8769 designs against.
Recommendation: (a) — the artifact and its contract are the design's current step; the interface is the next one, and landing it in a focused change keeps both reviewable. The thread is left open for that decision.
中文说明
已在当前提交上核实——三堵墙都真实存在：
WORKFLOW_PARAM_SCHEMA（packages/core/src/tools/workflow/workflow.ts）没有argsPath参数；args只接受内联的结构化值。scriptPath经resolveSavedWorkflowScript→readWorkflowFileSecurely（packages/core/src/agents/runtime/workflow-saved.ts）解析，会拒绝 saved-workflow 目录之外的任何路径——任意的--out目录也不例外。workflowsEnabled默认为false（packages/core/src/config/config.ts），Workflow 工具只在启用时才注册。
因此该发现成立：打印出的交接契约在当前构建的 Workflow 工具上无法履行。我没有擅自定死修复方向，因为这是范围/时序决策，不是代码问题：
- （a）保持本 PR 只做 emit（其声明的范围："目前没有任何东西路由到它"），把该产物所假定的 Workflow 工具接口——
argsPath参数，加上 review 产出脚本在 saved-workflow 目录之外的可信来源，以isWorkflowsEnabled()作门禁——作为 Proposal: rebuild /review Step 3–5 orchestration on the workflow engine #8769 的下一步、在接线之前单独落地。这让 core 包的接口变更保持在自己独立的改动里。 - （b）扩大本 PR，现在就把该接口落地。那是
packages/core的工具 schema 与脚本加载策略变更，超出了本 PR 声明的 emit-only 意图。 - （c）弱化 stdout 文本，让它不再描述工具尚无法履行的调用——代价是重写 Proposal: rebuild /review Step 3–5 orchestration on the workflow engine #8769 设计所依赖的交接契约。
建议：(a)——产物与契约是设计的当前一步，接口是下一步，单独落地可以让两者都可评审。线程保持打开，等待该决策。
| 'bug in the CLI, not in the call.', | ||
| ); | ||
| } | ||
| return { key, label: rosterLabel(req), prompt }; |
There was a problem hiding this comment.
[Suggestion] R1-3: WorkflowAgentSpec.label is computed via rosterLabel() and serialized into every args file, but has zero read sites: the emitted script — the only consumer of args.json — dispatches with agent(a.prompt, { label: a.key, ... }), and the runtime's progress display consumes opts.label. The doc comment ("Human-readable identity, for the run's progress display") describes a use no code performs. Project rule (AGENTS.md): "For every added field, option, or optional parameter, grep its read sites."
Concrete cost: every args file carries ~13 sanitized label strings nothing reads; the progress display shows terse roster keys (1a, 3b, 7) while the field's doc promises it the human-readable identity; a maintainer who changes or localizes rosterLabel output (exported by this same PR) expecting the workflow run's display to change sees no effect.
Suggested fix: dispatch with the carried label — agent(a.prompt, { label: a.label, phase: 'Review' }) in workflow-script.ts, updating the test that pins label to the key — or drop the field from WorkflowAgentSpec until a consumer exists.
中文说明
WorkflowAgentSpec.label 由 rosterLabel() 计算并写入每个 args 文件，但没有任何读取方：唯一消费 args.json 的脚本在分发时用的是 agent(a.prompt, { label: a.key, ... })，而运行时的进度展示消费的是 opts.label。字段注释（"Human-readable identity, for the run's progress display"）描述了一个没有任何代码实现的用途。项目规则（AGENTS.md）："对每个新增字段、选项、可选参数，都要 grep 它的读取方"。
具体代价：每个 args 文件都携带约 13 条无人读取的、已清洗的 label 字符串；进度展示显示的是简短的 roster key（1a、3b、7），而字段注释承诺的是人类可读的名称；维护者若修改或本地化 rosterLabel 的输出（本 PR 同时导出了它），以为 workflow 运行的展示会随之变化，实际不会有任何变化。
建议修复：在 workflow-script.ts 中分发时携带该 label——agent(a.prompt, { label: a.label, phase: 'Review' })，并更新当前把 label 钉为 key 的测试；或者在出现消费方之前，从 WorkflowAgentSpec 中移除该字段。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| /** Bumped when the script's expectations of this file change. */ | ||
| version: 1; |
There was a problem hiding this comment.
[Suggestion] R1-4: WorkflowArgsFile.version is written (line 143: return { version: 1, plan: planPath, mode, agents }) and asserted in the test, but read by nothing — the string version never appears in the script constant and the runtime passes args through opaquely, so the compatibility protocol the doc comment implies has no enforcement point. That is against this file's own stated philosophy of "refusals rather than fallbacks".
Concrete cost: when the payload shape next changes, the author bumps version per the comment; no consumer checks it, so a stale args.json left in a reused --out dir against a rebuilt script (e.g. a CI retry after a CLI upgrade where only one of the two files is regenerated) misreads the payload silently — args.agents undefined or carrying missing fields — instead of failing closed.
Suggested fix: have the script fail closed on a mismatch (string-concatenation style per the file's constraints), or drop the field until a reader exists:
if (args.version !== 1) {
throw new Error('args version ' + args.version + ' does not match this script - re-run emit-workflow');
}中文说明
WorkflowArgsFile.version 被写入（第 143 行：return { version: 1, plan: planPath, mode, agents }）并在测试中断言，但没有任何读取方——脚本常量中完全没有出现 version 字样，运行时也只是透明地传递 args，因此注释所暗示的兼容协议没有任何执行点。这与本文件自己声明的 "拒绝而非降级"（refusals rather than fallbacks）原则相悖。
具体代价：下次 payload 结构变化时，作者按注释把 version 加一；但没有任何消费方检查它，于是残留在复用 --out 目录里的旧 args.json 配上重建后的脚本（例如 CLI 升级后的 CI 重试只重新生成了两个文件之一）会静默地误读 payload——args.agents 为 undefined 或缺字段——而不是 fail closed。
建议修复：让脚本在版本不匹配时 fail closed（按本文件的约束用字符串拼接风格），或在出现读取方之前移除该字段（见上方代码）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| if (key !== req.key) { | ||
| throw new Error( | ||
| `emit-workflow: built "${key}" where the roster requires "${req.key}" ` + |
There was a problem hiding this comment.
[Suggestion] R1-5: The roster→buildLaunch→key-match loop is a second implementation of runRoster's mapping (agent-prompt.ts:1739-1766), guarded by the same key-parity invariant — two copies of one guard, the exact shape the new buildLaunch doc comment warns against ("a third caller that rebuilt this would be a second implementation of the invariant"). The implementations already differ: runRoster handles chunk specs and calls recordPrompt; this loop omits both — and that omission is not hypothetical drift, it is the live divergence behind R1-1.
Concrete cost: if key derivation or guard semantics ever change (e.g. the role--file key format for invariant agents), both files must change together; missing one makes --roster fail closed while emit-workflow silently emits agents whose keys check-coverage can never match — the exact false "brief never reached an agent" failure the guard exists to prevent, on only one dispatch path. The #8769 next dispatcher would copy the guard a third time.
Suggested fix: extract one mapper beside buildLaunch (e.g. buildRosterLaunches(report, planPath, rules)) owning requiredAgents → buildLaunch → the key guard; runRoster and buildWorkflowArgs both consume it, each adding its caller-specific concerns (recordPrompt / chunk refusal).
中文说明
roster→buildLaunch→key 匹配的循环是 runRoster 映射逻辑（agent-prompt.ts:1739-1766）的第二份实现，由同一个 key 一致性不变量守护——同一个守卫有了两份拷贝，正是新增的 buildLaunch 文档注释所警告的形态（"第三个重建此逻辑的调用方将构成该不变量的第二份实现"）。两份实现已经出现差异：runRoster 处理 chunk spec 并调用 recordPrompt；本循环两者都省略了——这个省略不是假想的漂移，而是 R1-1 背后的现实分歧。
具体代价：一旦 key 推导或守卫语义发生变化（例如 invariant agent 的 role--file key 格式），两个文件必须同步修改；漏改其一，--roster 会 fail closed，而 emit-workflow 会静默发出 check-coverage 永远无法匹配的 agent key——正是该守卫要防止的 "brief never reached an agent" 误报，且只发生在其中一条分发路径上。#8769 的下一个 dispatcher 将第三次复制这个守卫。
建议修复：在 buildLaunch 旁边抽出一个映射函数（例如 buildRosterLaunches(report, planPath, rules)），统一持有 requiredAgents → buildLaunch → key 守卫；runRoster 与 buildWorkflowArgs 都消费它，各自附加调用方特有的逻辑（recordPrompt / chunk 拒绝）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| it('reports the review mode it built for', () => { | ||
| expect(buildWorkflowArgs(localPlan(), planPath).mode).toBe('local'); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R1-9: The diff-only review mode never passes through buildWorkflowArgs in any test: every fixture is a local plan (untrackedFiles: []), and the single mode assertion uses a local plan. Mutant physically verified: hardcoding mode: 'local' in the return (emit-workflow.ts:143) survives all 10 tests.
Concrete cost: a cross-repo lightweight review (the plan-diff path, which reviewMode classifies as diff-only) would emit args.json declaring "mode": "local" without any test noticing — misdescribing the run's capabilities (e.g. implying a tree available for build/grep) in every record derived from the artifact. roster.test.ts covers reviewMode itself; the gap is emit-workflow's wiring of the diff-only shape, including its reduced roster (1c/7 absent).
Suggested fix:
expect(
buildWorkflowArgs(localPlan({ untrackedFiles: undefined }), planPath).mode,
).toBe('diff-only');中文说明
diff-only review 模式在任何测试中都没有流经 buildWorkflowArgs：所有 fixture 都是 local plan（untrackedFiles: []），唯一的 mode 断言也用的是 local plan。已实际做变异验证：把返回值里的 mode 硬编码为 'local'（emit-workflow.ts:143），全部 10 个测试依然通过。
具体代价：一次跨仓库轻量 review（plan-diff 路径，reviewMode 将其归类为 diff-only）会产出声明 "mode": "local" 的 args.json 而没有任何测试发现——在由该产物派生的每一条记录中错误描述该运行的能力（例如暗示存在可用于 build/grep 的代码树）。roster.test.ts 覆盖了 reviewMode 本身；缺口在 emit-workflow 对 diff-only 形态的接线，包括它缩减后的 roster（不含 1c/7）。
建议修复：见上方代码——补一个 diff-only 形态的用例。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const agents = args.agents; | ||
| log(agents.length + ' agents required by the plan'); |
There was a problem hiding this comment.
[Suggestion] R1-10: The emitted script performs no shape validation on args. Probe-reproduced by executing the real constant with three malformed bindings — args = undefined, args = '/tmp/out/args.json' (a path string — the exact R1-2 mix-up), and args without an agents key: all three crash with an opaque vm TypeError (Cannot read properties of undefined) naming nothing about the args file or the emit-workflow command.
Concrete cost: the debugger of a failed workflow run is left to guess between a script bug, a tool-wiring bug, and a bad args file — an opaque sandbox crash whose cause lives in a different command's stdout text.
Suggested fix (guard at the top of the script body; plain quotes only — the script forbids backticks):
| const agents = args.agents; | |
| log(agents.length + ' agents required by the plan'); | |
| if (!args || !Array.isArray(args.agents)) { | |
| throw new Error('review-step-3a: args.agents is missing or not an array - pass the args.json that qwen review emit-workflow wrote'); | |
| } | |
| const agents = args.agents; | |
| log(agents.length + ' agents required by the plan'); |
中文说明
产出的脚本没有对 args 做任何形状校验。已用真实常量实际执行复现：三种错误绑定——args = undefined、args = '/tmp/out/args.json'（路径字符串——正是 R1-2 的混淆情形）、以及缺少 agents 键的 args——全部以不透明的 vm TypeError（Cannot read properties of undefined）崩溃，错误信息中完全不提 args 文件或 emit-workflow 命令。
具体代价：一次 workflow 运行失败后，排查者只能在脚本 bug、工具接线 bug、坏 args 文件之间猜测——一个不透明的沙箱崩溃，而原因却藏在另一条命令的 stdout 文本里。
建议修复：在脚本体开头加守卫（见上方 suggestion；注意只能用普通引号——脚本禁止反引号）。探针验证：加上守卫后三种错误绑定都变为可操作的错误信息，原有 7 个正常路径测试仍全绿。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 0, | ||
| REVIEW_STEP_3A_WORKFLOW_SCRIPT.indexOf('\n};') + 3, | ||
| ); | ||
| expect(metaBlock).not.toMatch(/\$\{|\bfunction\b|\(\s*\)|\.\.\./); |
There was a problem hiding this comment.
[Suggestion] R1-11: The meta pure-literal assertion only detects ${, the function keyword, empty-paren calls, and spread — identifier references and calls-with-arguments pass it, so the contract its own comment states ("must contain no variables, calls") is not actually pinned.
Failure scenario (probe-confirmed against the real extractAndStripMeta from core): a future edit introducing description: 'Review Step 3A: ' + args.mode into the meta literal contains none of the regex's shapes, so this test and the whole CLI suite pass green — but the sandbox evaluates meta in a bare vm context with a null-prototyped globalThis and no bridge globals, so dispatch dies with extractAndStripMeta: failed to evaluate meta object literal: ReferenceError: args is not defined before any agent launches. That is exactly the runtime failure the test's comment ("the runtime reads it before executing anything") claims to prevent; the sandbox's own regression test (workflow-sandbox.test.ts:246) pins that throw.
Suggested fix (verified: flips the mutant to failing, original script still passes): do what the runtime does — slice the meta object literal, evaluate (${metaSource}) in a bare node:vm context (vm.createContext(Object.create(null))), and assert it yields the expected plain object (name, description, phases[].title).
中文说明
meta "纯字面量" 断言只能检出 ${、function 关键字、空括号调用和 spread——标识符引用和带参数的调用都能通过它，因此该测试注释自己声明的契约（"不得包含变量、调用"）实际上并未被钉住。
失败场景（已用 core 中真实的 extractAndStripMeta 探针验证）：未来若在 meta 字面量中引入 description: 'Review Step 3A: ' + args.mode，它不含该正则覆盖的任何形态，于是本测试乃至整个 CLI 测试套件全绿——但沙箱会在裸 vm 上下文（globalThis 无原型、无 bridge 全局变量）中求值 meta，分发会在任何 agent 启动之前就以 extractAndStripMeta: failed to evaluate meta object literal: ReferenceError: args is not defined 终止。这正是该测试注释（"运行时在执行任何内容之前先读取它"）声称要防止的运行时失败；沙箱自己的回归测试（workflow-sandbox.test.ts:246）钉住了这个抛错。
建议修复（已验证：可使上述变异体失败，原脚本仍通过）：做运行时所做的事——切出 meta 对象字面量，在裸 node:vm 上下文（vm.createContext(Object.create(null))）中求值 (${metaSource})，断言它产出预期的纯对象（name、description、phases[].title）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| * test asserts rather than something the code cannot break. | ||
| */ | ||
| function buildLaunch( | ||
| export function buildLaunch( |
There was a problem hiding this comment.
[Suggestion] R1-12: Test-efficacy probe (harness validated): reverting this hunk — the export on buildLaunch plus the rewritten byte-parity comment — on its own left every probed test green. The change ships with nothing in the collocated suite gating it; the only in-diff pin is emit-workflow.test.ts' import + byte-parity test, which the probe's affected set did not exercise.
Concrete cost: a future change that undoes this hunk or the invariant it documents (e.g. gives emit-workflow its own prompt builder) keeps the collocated suite green at change time, and the byte drift between the workflow fan-out and the hand-launched --roster path surfaces only in production, where the delivery check reads a correctly-run workflow review as "the prompt was rewritten".
Suggested fix: either confirm emit-workflow.test.ts' byte-parity test is considered the gate for this hunk, or add an assertion in the collocated agent-prompt.test.ts that the exported builder is what emit-workflow consumes.
中文说明
测试有效性探针（harness 已验证）：单独还原这个 hunk——buildLaunch 的 export 加上重写后的字节一致性注释——所有被探测的测试依然全绿。该改动上线时没有任何同目录（collocated）测试为其把关；diff 内唯一的钉住来自 emit-workflow.test.ts 的导入 + 字节一致性测试，而探针的影响集没有运行到它。
具体代价：未来若有改动撤销这个 hunk 或它所记录的不变量（例如给 emit-workflow 一个自己的 prompt 构建器），同目录测试套件在改动当时仍为绿色，workflow 扇出与手工发射 --roster 路径之间的字节漂移只会在生产环境暴露——届时交付检查会把一次正确运行的 workflow review 读成 "prompt 被改写"。
建议修复：要么确认 emit-workflow.test.ts 的字节一致性测试被视为该 hunk 的把关者，要么在同目录的 agent-prompt.test.ts 中补一条断言，验证导出的构建器就是 emit-workflow 所消费的。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Declined (no code change) with evidence: the gate for this hunk exists and is exercised — the probe's affected set simply did not include it. emit-workflow.test.ts imports buildLaunch from ./agent-prompt.js for its byte-parity test, and the production/test compilation also depends on the export. Mutation-verified at this commit: removing the export makes the byte-parity test fail (TypeError: buildLaunch is not a function), and npm run typecheck fails too (the test file is inside the typechecked src/**/*.ts include); reverting restores green. So we confirm the first option the finding offers: emit-workflow.test.ts' byte-parity test IS the gate for this hunk, by deliberate placement — the parity assertion lives with the consumer that depends on it.
中文说明
拒绝修改（不动代码），附证据：这个 hunk 的把关测试存在且会运行——只是探针的影响集没有包含它。emit-workflow.test.ts 从 ./agent-prompt.js 导入 buildLaunch 用于其字节一致性测试，生产/测试代码的编译也依赖该导出。已在本提交上做变异验证：去掉 export 后字节一致性测试失败（TypeError: buildLaunch is not a function），npm run typecheck 同样失败（该测试文件在被类型检查的 src/**/*.ts include 之内）；还原后恢复全绿。因此确认该发现提供的第一个选项：emit-workflow.test.ts 的字节一致性测试就是这个 hunk 的把关者，且是刻意放置的——一致性断言与依赖它的消费方放在一起。
| * spaces, and the separator glyph is stripped so a name cannot imitate one. | ||
| */ | ||
| function rosterLabel(req: RequiredAgent): string { | ||
| export function rosterLabel(req: RequiredAgent): string { |
There was a problem hiding this comment.
[Suggestion] R1-13: Test-efficacy probe (harness validated): reverting this hunk — the export on rosterLabel — on its own left every probed test green. emit-workflow.ts writes label: rosterLabel(req) into every agent record, but no test asserts anything about label (and the script labels dispatches by a.key — see R1-3).
Concrete cost: a future refactor of emit-workflow.ts:140 substituting a different/empty label source keeps every test green, so the workflow args file silently carries wrong agent display names and any consumer keyed on label misidentifies agents with no test signal at the point of divergence.
Suggested fix: add an assertion in emit-workflow.test.ts that each emitted agent's label matches rosterLabel's format for that requirement (e.g. the role label shape).
中文说明
测试有效性探针（harness 已验证）：单独还原这个 hunk——rosterLabel 的 export——所有被探测的测试依然全绿。emit-workflow.ts 把 label: rosterLabel(req) 写入每一条 agent 记录，但没有任何测试对 label 做断言（而且脚本分发时用的是 a.key 作标签——见 R1-3）。
具体代价：未来若重构 emit-workflow.ts:140、改用其他/空的 label 来源，所有测试仍为绿色，workflow args 文件会静默携带错误的 agent 展示名称，任何以 label 为键的消费方都会在分叉点上误认 agent，且得不到任何测试信号。
建议修复：在 emit-workflow.test.ts 中补一条断言，验证每个产出 agent 的 label 符合 rosterLabel 对该要求的格式（例如 role 标签形态）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| 'bug in the CLI, not in the call.', | ||
| ); | ||
| } | ||
| return { key, label: rosterLabel(req), prompt }; |
There was a problem hiding this comment.
buildWorkflowArgs never calls recordPrompt, so the coverage gate sees the whole roster as unlaunched.
runRoster (agent-prompt.ts, roster loop ~1743-1765) calls recordPrompt(planPath, key, prompt) for every agent. buildWorkflowArgs copies that loop but drops the call — it only returns { key, label, prompt }, and the file does not even import recordPrompt.
lib/coverage.ts reads those records via readRecordedPrompts(planPath) to compute builtOf(key). With no .txt records the map is empty, so nobodyBuiltAnything (roster.length > 1 && every role briefless) is true. A review run entirely through emit-workflow — where all 11–13 agents actually ran and delivered findings — makes check-coverage emit the collapsed disclosure "every dimension — none of the N required agents is on record as launched with a prompt this skill built", exit 3, and compose-review renders that line as the CHANGES_REQUESTED body.
The fan-out is byte-identical to --roster, yet the run is reported to the PR author as entirely unreviewed.
| // CLI computed; this loop cannot shorten it, and there is no branch in which | ||
| // an agent is skipped. | ||
| const returns = await parallel( | ||
| agents.map((a) => () => agent(a.prompt, { label: a.key, phase: 'Review' })), |
There was a problem hiding this comment.
Workflow-dispatched agents write no subagent JSONL, which is the only evidence check-coverage accepts that an agent ran.
coverage.ts calls readTranscripts(mtimeMs, env, plan.diffPathAbsolute), which reads <QWEN_CODE_PROJECT_DIR>/subagents/<QWEN_CODE_SESSION_ID>/*.jsonl (transcripts.ts:99-109). Those files are written only by the Agent tool — getAgentJsonlPath is called solely in packages/core/src/tools/agent/agent.ts and background-agent-resume.ts.
This dispatch is a bare agent(a.prompt, { label, phase }), so the runtime takes the workflow fast path: createProductionDispatch builds an AgentHeadless directly and never wires a transcript path. A fan-out dispatched by this script therefore produces zero transcripts — readTranscripts either throws TranscriptsUnavailableError ("no subagent transcripts at …") when the dir does not exist, or returns an empty array so every roster entry reads as not-launched.
Together with the missing recordPrompt, Step 3A halts at the coverage gate no matter how well the agents performed.
| mkdirSync(outDir, { recursive: true }); | ||
| const scriptPath = resolve(outDir, 'script.js'); | ||
| const argsPath = resolve(outDir, 'args.json'); | ||
| writeFileSync(scriptPath, REVIEW_STEP_3A_WORKFLOW_SCRIPT, 'utf8'); |
There was a problem hiding this comment.
script.js is written into an arbitrary --out directory, but Workflow({scriptPath}) refuses to load any file outside the saved-workflow dirs.
resolveSavedWorkflowScript (packages/core/src/agents/runtime/workflow-saved.ts:222) routes every scriptPath through readWorkflowFileSecurely, which realpaths the file and rejects it unless it sits under <repo>/.qwen/workflows or ~/.qwen/workflows (workflow-saved.ts:167-172, getSavedWorkflowDirs :84-88).
--out is an unconstrained yargs string with no validation and no steering — the tests use mkdtempSync(tmpdir()), and the review skill's scratch dirs live under .qwen/tmp/. An orchestrator that follows the printed scriptPath: line gets:
workflow({scriptPath: '/…/out/script.js'}): refusing to load a workflow file outside the saved-workflow directories
Step 3A launches zero agents and the review stalls at the fan-out.
| 'calls by hand for this step.', | ||
| ); | ||
| writeStdoutLine(`scriptPath: ${scriptPath}`); | ||
| writeStdoutLine(`args: ${argsPath}`); |
There was a problem hiding this comment.
The printed args: <path> line is not something the Workflow tool can consume — args takes inline JSON, and there is no argsPath.
WORKFLOW_PARAM_SCHEMA (packages/core/src/tools/workflow/workflow.ts:33-58) accepts script XOR scriptPath, plus args, documented as "Optional structured value bound to the args global. Pass actual JSON, not a stringified value." Nothing in core reads an args.json from disk — grepping for args.json/argsPath finds no reader.
An orchestrator that follows "make one Workflow call with the paths below" literally calls Workflow({scriptPath: '…/script.js', args: '…/args.json'}). Inside the sandbox args is then the path string, args.agents is undefined, and agents.length in the emitted script throws TypeError: Cannot read properties of undefined (reading 'length'). Zero agents dispatched.
It also undercuts the stated premise that the model "never sees a prompt": the only call that actually works requires reading args.json and inlining all ~13 prompts into the tool call.
| * compares agents against records — a drift between the two paths would read as a | ||
| * rewritten launch on a run that did everything right. | ||
| * One body for every caller on purpose: the single-agent path, `--roster` and | ||
| * `emit-workflow` must emit byte-identical prompts for the same agent, because |
There was a problem hiding this comment.
Byte-identical prompts do not mean identical agents: workflow dispatch replaces the system prompt with one that orders the agent to be terse.
This docstring's invariant is that --roster and emit-workflow launch the same agent. But the emitted script dispatches with only label/phase, which takes the workflow fast path, and that path overrides the system prompt with WORKFLOW_SUBAGENT_SYSTEM_PROMPT (workflow-orchestrator.ts:437-439) — it tells the agent its final text is a return value to a script and ends with "Be concise. The script will parse your output."
Review briefs ask for multi-field findings with file/line/failure-scenario prose. The same roster run through the workflow yields shorter, thinner findings than --roster for the same diff. Since the PR's stated purpose is an A/B comparing two dispatchers, the comparison silently varies the agents' output contract as well as who launches them.
There was a problem hiding this comment.
Escalated to maintainer — not fixable from this PR's emitted artifact. Verified at the code: the terseness directive comes from the workflow runtime's fast path — WORKFLOW_SUBAGENT_SYSTEM_PROMPT (workflow-orchestrator.ts) replaces the general-purpose system prompt on dispatches without agentType, and KNOWN_AGENT_OPTS in workflow-sandbox.ts offers no opt-out. So the same roster run through the workflow really does get a different output contract than --roster; that is a property of the runtime, not of the script this command emits.
Options as I see them:
- Accept and document that this A/B compares dispatcher + runtime policy as one bundle (cheapest; the comparison stays internally valid, just narrower than the stated premise).
- Add a runtime opt that keeps the general-purpose system prompt for dispatched review agents (core change in workflow-orchestrator.ts; also needed for the per-agent caps in the sibling thread).
- Hold the A/B until (2) lands.
My recommendation: (1) now, with (2) tracked as a follow-up — but this is an experiment-design call, so I am leaving the question open: which of the three do you want?
中文说明
升级给维护者——无法在本 PR 的发出物层面修复。 已在代码中核实：简洁化指令来自 workflow 运行时的快速路径——WORKFLOW_SUBAGENT_SYSTEM_PROMPT（workflow-orchestrator.ts）在不带 agentType 的分发中替换通用系统提示，而 workflow-sandbox.ts 的 KNOWN_AGENT_OPTS 不提供退出选项。因此同一份 roster 经 workflow 运行时的确会得到与 --roster 不同的输出契约；这是运行时的属性，不是本命令所发脚本的属性。
我看到的选项：
- 接受并声明：本 A/B 比较的是分发器 + 运行时策略这一整体（成本最低；比较仍然内部有效，只是范围比声明的前提更窄）。
- 为分发的评审 agent 增加保留通用系统提示的运行时选项（workflow-orchestrator.ts 的 core 改动；姊妹线程中的每 agent 上限也需要它）。
- 在 (2) 落地前暂缓该 A/B。
我的建议：先按 (1) 执行，并把 (2) 作为后续工作跟踪——但这属于实验设计决策，因此问题保持开放：三者之中您希望选哪一个？
| 'bug in the CLI, not in the call.', | ||
| ); | ||
| } | ||
| return { key, label: rosterLabel(req), prompt }; |
There was a problem hiding this comment.
WorkflowAgentSpec.label has no read site — the script labels agents by key.
The field is documented as "Human-readable identity, for the run's progress display", but workflow-script.ts:48 dispatches agent(a.prompt, { label: a.key, phase: 'Review' }). The orchestrator's emitter.agentDispatched(label) therefore feeds the TUI raw roster keys (1a, 3c, invariant-a--src/x.ts) instead of Agent 1a: Line-by-line correctness — exactly the display this field was added for.
Grepping .label across the diff and the repo finds no consumer of WorkflowAgentSpec.label; the rosterLabel export newly added to agent-prompt.ts exists only to populate it. Either pass label: a.label in the script, or drop the field and the export.
| return { key, label: rosterLabel(req), prompt }; | ||
| }); | ||
|
|
||
| return { version: 1, plan: planPath, mode, agents }; |
There was a problem hiding this comment.
WorkflowArgsFile.version is written but never read, so the compatibility check it claims to provide does not exist.
The field is documented "Bumped when the script's expectations of this file change", but REVIEW_STEP_3A_WORKFLOW_SCRIPT reads only args.agents — it never inspects args.version. The sole read site in the repo is expect(args.version).toBe(1) in emit-workflow.test.ts.
If the payload shape later changes and the version is bumped, an older script.js sitting in a stale --out directory still runs against the new args and fails on the shape rather than on the version — precisely the outcome the field was added to prevent. Per AGENTS.md, a field whose only reader is its own test is a dead switch: either have the script assert the version, or drop the field.
|
|
||
| phase('Review'); | ||
|
|
||
| const agents = args.agents; |
There was a problem hiding this comment.
args.agents is dereferenced with no shape check, so a malformed Workflow call dies on an anonymous TypeError.
Because the payload has to be inlined by hand into the Workflow call (there is no argsPath — see the args: <path> comment), an orchestrator that passes args: {}, forgets the key, or passes the path string gets:
TypeError: Cannot read properties of undefined (reading 'length')
with no mention of args.json, the roster, or emit-workflow. The review step fails with a message that points at the sandbox rather than at the malformed call. A one-line guard (if (!Array.isArray(args?.agents)) throw new Error('review workflow: args.agents must be the array written by \qwen review emit-workflow`')`) turns this into a diagnosable failure.
| log(missingRoles.length + ' agent(s) returned nothing: ' + missingRoles.join(', ')); | ||
| } | ||
|
|
||
| return { |
There was a problem hiding this comment.
Collapsing the whole roster into one workflow run puts every agent behind a single wall-clock cap with no partial-result path.
The sandbox arms a wall-clock watchdog (default 30 min, workflow-sandbox.ts:~1120/1155) that aborts and rejects the whole run. Since this script only returns after parallel() settles, a high-effort review whose slowest agent runs past the cap loses every already-completed agent's findings — the run rejects with Workflow execution exceeded … ms and delivers nothing.
The --roster path has no such all-or-nothing failure: each agent returns to the orchestrator independently as it finishes, so a single slow agent costs one dimension, not the review. Worth either raising/plumbing the cap for this run or checkpointing per-agent results as they settle.
There was a problem hiding this comment.
Escalated to maintainer — core-runtime decision, same cluster as the per-agent cap threads. Verified: the sandbox arms a wall-clock watchdog (default 30 min, QWEN_CODE_MAX_WORKFLOW_SECONDS) that rejects the whole run, and this script returns only after parallel() settles — so a single slow agent past the cap discards every already-completed agent's findings, where --roster loses one dimension at most. That asymmetry is real.
What this round did within the artifact: a zero-delivery fan-out now throws instead of returning a clean shape (so total failure is at least legible), and partial failures still return their delivered results. What the artifact cannot do: checkpoint per-agent results as they settle — the sandbox has no filesystem, and log() output on a rejected run is not a retrieval path the skill can consume. So the substantive fixes are runtime-side: raising/plumbing the wall-clock cap for review fan-outs, or a journal/snapshot path that survives an abort (both also serve #8769's journal-based coverage). Recommendation: track with the sibling threads as one follow-up; maintainer decision requested there.
中文说明
升级给维护者——core 运行时决策，与每 agent 上限线程同属一簇。 已核实：沙箱装有墙钟看门狗（默认 30 分钟，QWEN_CODE_MAX_WORKFLOW_SECONDS），到点即拒绝整个运行；而本脚本只在 parallel() 全部落定后才返回——因此一个慢 agent 越过上限会丢弃所有已完成 agent 的发现，而 --roster 至多损失一个维度。这个不对称是真实的。
本轮在发出物范围内做到的：零交付的扇出现在会抛错而不是返回一个形状"干净"的结果（至少让整体失败可读）；部分失败仍正常返回已交付结果。发出物做不到的：在每个 agent 落定时做检查点——沙箱没有文件系统，被拒运行的 log() 输出也不是 skill 可消费的回取路径。因此实质修复在运行时侧：为评审扇出提高/接通墙钟上限，或提供能在中止后留存的 journal/snapshot 路径（两者同时服务 #8769 的 journal 版 coverage）。建议：与姊妹线程合并为一个后续工作跟踪；在那里请求维护者决策。
| writeStdoutLine(`args: ${argsPath}`); | ||
| } | ||
|
|
||
| export const emitWorkflowCommand: CommandModule = { |
There was a problem hiding this comment.
emit-workflow has no caller — no skill, doc, or code outside its own two files references it.
git grep -n "emit-workflow\|emitWorkflow" on the PR branch hits only review.ts:28 (import), review.ts:61 (registration), and the feature's own files/tests. .qwen/skills/** — including the review SKILL.md that actually drives Step 3A — docs, and every other code path never invoke it.
Combined with the pr-worktree refusal (which excludes every PR review), the subcommand is reachable only by hand on a local uncommitted diff. The CLI's public subcommand list and help string grow by an entry that no supported flow can use, and the dead-code check for the whole feature reduces to "the tests call it". If this is intentionally staged ahead of the skill change, saying so in the PR body (or gating it behind an env flag until the skill lands) would make that legible.
Code review — 10 findings (high effort)Ten inline comments are on the diff. Summary of what they add up to: The
(3) and (4) mean the fan-out errors before dispatching a single agent; (1) and (2) mean that even if it dispatched, a run in which all 11–13 agents delivered findings is reported to the PR author as entirely unreviewed — collapsed disclosure, exit 3, CHANGES_REQUESTED. Note (4) also undercuts the "the model never sees a prompt" premise: the only Workflow call that actually works requires inlining all ~13 prompts into the tool call. The dispatcher-only A/B is not clean. Byte-identical prompts do not mean identical agents — the workflow fast path substitutes Cleanup: |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review round summary (PR #8846)Commit: Resolved in code
Declined with evidence
Escalated for a maintainer decision (thread left open, reply posted)
Review-level items (not inline findings)
No base-conflict merge was performed ( VerificationCommands actually run, in order:
中文说明评审轮次总结（PR #8846）提交： 已在代码中解决
附证据拒绝
升级给维护者决策（线程保持打开，已回复）
评审级条目（非行内发现）
未执行 base 冲突合并（ 验证实际执行的命令，按顺序：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
|
||
| const outDir = resolve(args.out); | ||
| mkdirSync(outDir, { recursive: true }); | ||
| const scriptPath = resolve(outDir, 'script.js'); |
There was a problem hiding this comment.
script.js is written outside the saved-workflow directories the Workflow loader accepts.
The script goes into the arbitrary --out directory and the path is printed to the orchestrator as a Workflow scriptPath. But WorkflowRunner.start → resolveSavedWorkflowScript({scriptPath}) → readWorkflowFileSecurely computes
const inside = dirs.some((d) => real === d || real.startsWith(d + sep));over getSavedWorkflowDirs() (workflow-saved.ts:167), i.e. only <project>/.qwen/workflows and ~/.qwen/workflows. Any other --out throws:
workflow({scriptPath: '…/script.js'}): refusing to load a workflow file outside the saved-workflow directories
So the single Workflow call this command instructs the orchestrator to make fails before dispatching anything — Step 3A launches zero agents and the fan-out never starts. Either write the script into a saved-workflow dir, or ship it as a named saved workflow and print name: instead of scriptPath:.
| 'calls by hand for this step.', | ||
| ); | ||
| writeStdoutLine(`scriptPath: ${scriptPath}`); | ||
| writeStdoutLine(`args: ${argsPath}`); |
There was a problem hiding this comment.
args: <path> is not a form the Workflow tool accepts — args is inline-only.
WORKFLOW_PARAM_SCHEMA (packages/core/src/tools/workflow/workflow.ts:128) declares args as "Optional structured value bound to the args global. Pass actual JSON, not a stringified value", and there is no argsPath parameter anywhere in the codebase.
Following the printed instruction verbatim, the orchestrator passes args: "/…/args.json" and the emitted script's own guard fires — review-step-3a: args.agents is missing or not an array (the exact case workflow-script.test.ts enumerates) — so no agent runs.
The only way to actually dispatch from this output is to open args.json and retype ~13 full launch prompts into the tool call by hand, which is precisely the prompt-rewrite failure mode (rewrittenPrompts in check-coverage) this command exists to eliminate.
| // The delivery gate reads recorded prompts, not briefs: without this | ||
| // record, a run dispatched from these args would fail check-coverage as | ||
| // "briefless" despite reviewing with exactly the built prompts. | ||
| recordPrompt(planPath, key, prompt); |
There was a problem hiding this comment.
Recording the prompts is only half the gate — workflow-dispatched agents write no subagent transcript.
The comment here asserts that recordPrompt makes a workflow-dispatched run pass check-coverage. It does not. <projectDir>/subagents/<sessionId>/agent-*.jsonl is written only by attachJsonlTranscriptWriter, which is called solely from the Agent tool (packages/core/src/tools/agent/agent.ts:3190, :3969). The workflow path builds AgentHeadless directly in runSingleDispatch and attaches no transcript writer.
So after a successful workflow fan-out, qwen review check-coverage --plan … → readTranscripts either throws TranscriptsUnavailableError ("no subagent transcripts at …", the directory never being created) or matches no record, and every roster role lands in missingRoles as "its prompt was built, but no agent on record was launched with it". check-coverage exits 3 and the review cannot certify the diff — even though every agent ran with exactly the built prompts.
The delivery gate needs an evidence source the workflow path actually produces before this migration can land.
| // an agent is skipped. The label is the human-readable roster identity the | ||
| // args carry — the runtime's progress display consumes it. | ||
| const returns = await parallel( | ||
| agents.map((a) => () => agent(a.prompt, { label: a.label, phase: 'Review' })), |
There was a problem hiding this comment.
Workflow dispatch imposes a hard 50-turn / 10-minute cap per agent that the --roster (Agent tool) path does not, and a capped agent loses all its work.
agent(a.prompt, { label, phase }) with no agentType/model/isolation/schema takes the fast path in workflow-orchestrator.ts, which hard-wires max_turns: 50, max_time_minutes: 10 (constants at :152-153, applied at :447-448) and then throws Workflow subagent … did not complete (terminate mode: TIMEOUT|MAX_TURNS) for any non-GOAL terminal.
Agent 7 (Build & Test) runs the project build and full test suite, which exceeds 10 minutes on this repo. parallel() converts the thrown dispatch to null, the script folds it into missingRoles, and the agent's entire output — including everything it had already found — is discarded. The run still returns a result that reads as a completed fan-out with a one-line log, so the build/test dimension is silently unreviewed on exactly the large PRs where it matters most.
The --roster path launches through the Agent tool with no wall-clock cap and returns partial work, so this is a capability regression, not a wash.
There was a problem hiding this comment.
Escalated to maintainer — core-runtime decision, sibling of the system-prompt thread. Verified: WORKFLOW_SUBAGENT_MAX_TURNS = 50 and WORKFLOW_SUBAGENT_MAX_TIME_MINUTES = 10 are hard-wired in workflow-orchestrator.ts for both dispatch paths, and parallel() converts a capped/failed dispatch to null, so a capped agent's partial work is discarded. On a large repo, Agent 7 (Build & Test) can genuinely exceed 10 minutes, and --roster has no equivalent cap — so this is a real capability difference, not a wash.
It cannot be fixed from the emitted script: the caps are applied by the runtime around the dispatch, and KNOWN_AGENT_OPTS exposes no override. Options: (1) raise/plumb the caps for this workflow run only (core change), (2) accept the difference and document it as part of the measured variable, (3) hold the A/B. I recommend (2) now + a follow-up issue for (1), together with the sibling thread — but the call is the maintainer's; same question stands there.
中文说明
升级给维护者——core 运行时决策，与系统提示线程同族。 已核实：WORKFLOW_SUBAGENT_MAX_TURNS = 50 与 WORKFLOW_SUBAGENT_MAX_TIME_MINUTES = 10 在 workflow-orchestrator.ts 中对两条分发路径均为硬编码，且 parallel() 会把触顶/失败的分发转为 null，因此触顶 agent 的已完成工作会被丢弃。在大型仓库上，Agent 7（Build & Test）确实可能超过 10 分钟，而 --roster 没有等价上限——所以这是真实的能力差异，不是打平。
无法从发出的脚本修复：上限由运行时在分发外围施加，KNOWN_AGENT_OPTS 不暴露覆盖项。选项：(1) 仅为本 workflow 运行提高/接通上限（core 改动）；(2) 接受差异并将其作为被测变量的一部分如实记录；(3) 暂缓 A/B。我的建议：先按 (2)，并与姊妹线程合并开一个后续 issue 跟踪 (1)——但决定权在维护者；同一问题在那里同样开放。
|
|
||
| // Two paths and a count. Nothing here is a prompt: the prompts are in the | ||
| // args file, which no one is asked to read, retype, or relay. | ||
| writeStdoutLine( |
There was a problem hiding this comment.
No precondition check that workflows are enabled, and by this point every prompt record is already on disk.
Workflows are opt-in and experimental — Config.isWorkflowsEnabled (packages/core/src/config/config.ts:6474-6480) is off unless QWEN_CODE_ENABLE_WORKFLOWS=1 or the setting is on. This file has zero references to that gate; its only refusals are for territory fan-outs and pr-worktree.
buildWorkflowArgs has already called recordPrompt for all ~13 roles by the time this prints "make one Workflow call with the paths below and do not build agent calls by hand for this step", and the printed output deliberately contains no prompts. On a session without the Workflow tool the orchestrator has nothing to call and no prompt text to fall back on. If it then hand-builds launches, check-coverage compares them against the records already written and reports every role as rewritten or missing.
--roster had no such dependency — its output was self-sufficient. At minimum this should refuse up front (before any record is written) when workflows are disabled.
| } | ||
|
|
||
| if (missingRoles.length > 0) { | ||
| log(missingRoles.length + ' agent(s) returned nothing: ' + missingRoles.join(', ')); |
There was a problem hiding this comment.
A completely failed fan-out is logged and returned as a successful step result.
null dispatches are collected into missingRoles, a log() line is emitted, and the script returns normally with rosterSize/delivered/missingRoles. Nothing throws; nothing sets a failure status.
If the provider is overloaded and all 13 dispatches fail, the Workflow tool hands the orchestrator a well-formed result object — "13 agents required" plus a missingRoles list buried in a log line — and, since the coverage gate cannot see workflow agents at all (see the transcript comment on emit-workflow.ts), the run proceeds to Step 4/5 and composes a review from zero agent returns.
Under --roster, a failed Agent tool call surfaces per launch as an error the orchestrator has to act on and cannot roll into a successful step result. Consider throwing when delivered === 0, or when missingRoles is non-empty.
| // would run in the user's main checkout and review whatever is there — | ||
| // producing findings that look plausible and describe the wrong tree. | ||
| const mode = reviewMode(plan); | ||
| if (mode === 'pr-worktree') { |
There was a problem hiding this comment.
Refusing pr-worktree excludes the dominant review path; the real blocker is one layer down.
fetch-pr always creates a worktree, so reviewMode returns 'pr-worktree' for every PR review — the path this skill primarily exists for. With this refusal the new dispatcher can only ever run for uncommitted-local or cross-repo diff-only reviews, i.e. the minority of runs.
The deeper fix is in the mechanism the refusal names: agent()'s option allowlist (KNOWN_AGENT_OPTS = label, phase, schema, model, isolation, agentType, stallMs in workflow-sandbox.ts) has no working-directory option. Adding one there is a small change and unblocks the actual use case. Shipping the CLI half first means a second migration later plus a dead refusal branch to delete.
There was a problem hiding this comment.
Declined for this PR — recorded as follow-up. The diagnosis is right: the refusal excludes PR-worktree reviews, and the deeper fix is a working-directory option on agent(). But that is a core-runtime feature — new KNOWN_AGENT_OPTS surface in workflow-sandbox.ts plus dispatch/AgentHeadless cwd plumbing — and it decides which tree agents execute in, which deserves a dedicated security-minded review rather than a drive-by inside a review-tooling PR. Until it lands, the refusal is the correct fail-closed behavior: the alternative (dispatching anyway) reviews the wrong tree and looks complete doing it.
Suggestion: file the agent() working-directory capability as its own issue; when it exists, this PR's refusal branch is the thing to delete. I have left the refusal comment pointing at exactly that missing capability so the removal is obvious.
中文说明
本 PR 内拒绝——已记录为后续工作。 诊断是对的：该拒绝把 PR worktree 评审排除在外，更深层的修复是给 agent() 增加工作目录选项。但那是一个 core 运行时特性——workflow-sandbox.ts 中新的 KNOWN_AGENT_OPTS 表面，加分发/AgentHeadless 的 cwd 接线——它决定 agent 在哪棵树上执行，应当走一次专门的、带安全视角的评审，而不是顺手带进一个评审工具 PR。在其落地之前，拒绝是唯一正确的 fail-closed 行为：替代方案（照常派发）会评审错误的树，并且看起来完整。
建议：把 agent() 工作目录能力单独立一个 issue；它落地之时，本 PR 的拒绝分支就是要删除的对象。我已让拒绝注释恰好指向这个缺失能力，使将来的移除一目了然。
|
|
||
| function runEmitWorkflow(args: EmitWorkflowArgs): void { | ||
| let report: PlanReport; | ||
| try { |
There was a problem hiding this comment.
Plan-read / rules-read blocks are copy-pasted, making a fourth near-identical implementation in this directory.
The try { JSON.parse(readFileSync(...)) } catch { throw new Error('<cmd>: cannot read the plan …') } shape already exists verbatim at agent-prompt.ts:2280, build-test.ts:264 and script-lint.ts:532. The --rules block below is a shortened copy of agent-prompt.ts:2293-2305 — and this copy silently drops the trailing sentence explaining why a bad rules path is refused, so the error text has already drifted.
Any future change to plan validation (e.g. rejecting a plan that parses but has no diffPathAbsolute) now has to be applied in four places. A readPlan(path, command) / readRules(path, command) pair in lib/ would be one implementation.
| /** The plan these agents were derived from. */ | ||
| plan: string; | ||
| /** `local` or `diff-only` — `pr-worktree` is refused (see the header). */ | ||
| mode: string; |
There was a problem hiding this comment.
mode and plan are written into args.json but never read.
The emitted script (workflow-script.ts) reads only args.agents and args.version; grepping the branch finds no other reader of .mode or .plan outside emit-workflow.test.ts's own assertions. mode additionally costs a second reviewMode(plan) call, since requiredAgents already computes it internally.
These are declared-and-set-but-never-read fields — dead switches future readers have to reason about and keep in sync with a payload nothing consumes.
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8846 adds a qwen review emit-workflow subcommand th...: did not read the full 1838-line workflow-sandbox.ts , only the processing points the script depends on.; PR #8846 adds a qwen review emit-workflow subcommand th...: full emit-workflow.test.ts execution blocked on the missing node_modules ..
Test Plan (not a blocker): src/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no such file or directory.
中文说明
未探索到全部深度（达到工具调用预算）：PR #8846 adds a qwen review emit-workflow subcommand th...：did not read the full 1838-line workflow-sandbox.ts , only the processing points the script depends on.；PR #8846 adds a qwen review emit-workflow subcommand th...：full emit-workflow.test.ts execution blocked on the missing node_modules .。
Test Plan（非阻断）：src/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no such file or directory。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| writeStdoutLine(`scriptPath: ${scriptPath}`); | ||
| writeStdoutLine(`args: ${argsPath}`); |
There was a problem hiding this comment.
[Critical] R1-2: The printed handoff contract — "make one Workflow call with the paths below" — cannot be honored by the Workflow tool as printed. Re-verified at the reviewed commit, all three walls stand: (1) scriptPath outside .qwen/workflows or ~/.qwen/workflows is refused by readWorkflowFileSecurely (workflow-saved.ts:170); (2) WORKFLOW_PARAM_SCHEMA accepts inline args JSON only — there is no argsPath parameter for the printed args: <path> line; (3) workflowsEnabled defaults to false (config.ts), so a default install has no Workflow tool at all.
Failure scenario: a reviewer runs qwen review emit-workflow --plan plan.json --out .qwen/tmp/review-x and follows the printed instruction verbatim — on a default install there is no Workflow tool to call; with workflows enabled, the emitted script.js is refused because --out is not a saved-workflow dir, and the args: line has no parameter it can be passed through. The one-call dispatch the command promises is unexecutable as printed; the run falls back to hand-launching the roster, and the A/B this feature is built for never happens.
Suggested fix: make the emitted contract executable or refuse to emit it — validate --out/scriptPath against the saved-workflow dirs (or write into .qwen/workflows/), add an argsPath-style parameter to the Workflow tool, and gate on isWorkflowsEnabled(). Until then, the printed text should not describe a call the tool cannot honor.
中文说明
[Critical] R1-2：打印出来的交接契约（"make one Workflow call with the paths below"）按原样无法被 Workflow 工具执行。已在本评审提交上重新核实，三堵墙均成立：(1) .qwen/workflows 或 ~/.qwen/workflows 之外的 scriptPath 会被 readWorkflowFileSecurely（workflow-saved.ts:170）拒绝；(2) WORKFLOW_PARAM_SCHEMA 只接受内联 args JSON——不存在 argsPath 参数来承接打印出的 args: <path> 行；(3) workflowsEnabled 默认为 false（config.ts），默认安装根本没有 Workflow 工具。
失败场景：运行 qwen review emit-workflow --plan plan.json --out .qwen/tmp/review-x 并照打印输出执行——默认安装下无工具可调；启用 workflows 后，--out 不是 saved-workflow 目录导致 script.js 被拒，args: 行也没有可传入的参数。命令承诺的一次调用按原样不可执行；回退到手工发射 roster 后，本特性要做的 A/B 永远不会发生。
建议修复：让发出的契约可执行，或拒绝发出——校验 --out/scriptPath 位于 saved-workflow 目录（或自动写入 .qwen/workflows/），给 Workflow 工具增加 argsPath 风格参数，并用 isWorkflowsEnabled() 把关。在此之前，打印文本不应描述一个工具无法履行的调用。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| try { | ||
| report = JSON.parse(readFileSync(args.plan, 'utf8')) as PlanReport; | ||
| } catch (err) { |
There was a problem hiding this comment.
[Suggestion] runEmitWorkflow adds a 6th copy of the read-plan-or-throw block and a 2nd copy of the read-rules-or-throw block, duplicating logic that already exists in agent-prompt.ts, build-test.ts, test-plan.ts, script-lint.ts and base-tree.ts.
Failure scenario: the refusal semantics are now maintained in six copies that have already drifted (base-tree omits the command prefix; build-test alone validates the parsed value is a JSON object; emit-workflow's rules message truncates agent-prompt's explanation). Any future change to the refusal must be re-made in every copy, and each new command copies whichever variant is nearest, so the copies can only diverge further.
Suggested fix: extract a shared readPlanOrThrow(path, command) / readRulesOrThrow(path, command) into review/lib/ and call it from the new command — and opportunistically from the existing five, which is what makes the extraction pay for itself.
中文说明
[Suggestion] runEmitWorkflow 新增了第 6 份 read-plan-or-throw 代码块和第 2 份 read-rules-or-throw 代码块，与 agent-prompt.ts、build-test.ts、test-plan.ts、script-lint.ts、base-tree.ts 中已有的逻辑重复。
失败场景：拒绝语义现在由六份拷贝维护，且已经出现漂移（base-tree 省略命令前缀；只有 build-test 校验解析值是 JSON 对象；emit-workflow 的 rules 消息截断了 agent-prompt 的解释）。任何对拒绝逻辑的改动都要在每个拷贝中重做，新命令只会复制最近的变体，拷贝只会越来越分叉。
建议修复：把共享的 readPlanOrThrow(path, command) / readRulesOrThrow(path, command) 提取到 review/lib/，新命令调用它——并顺手改造现有五个调用点，提取才有收益。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const chunk = requiredAgents(plan).find((r) => r.role === 'chunk'); | ||
| if (chunk) { | ||
| throw new Error( |
There was a problem hiding this comment.
[Suggestion] The chunk-agent guard in buildWorkflowArgs is unreachable dead code — requiredAgents emits role: 'chunk' only inside its own isTerritoryFanOut branch in the same file (lib/roster.ts), and that same predicate already threw two statements above in this function.
Failure scenario: the trigger cannot be constructed through the public API: the only chunk-emission site in requiredAgents is gated on isTerritoryFanOut(plan), which this function already refuses first. The check is consequently also untestable (no fixture can trip it without mocking requiredAgents). The concrete cost is reader tax — a reader must trace roster.ts to discover the error "This is a bug in the CLI" can never be true today. (Agents 3c and 6c judged it a deliberate, commented future-change assertion consistent with the file's defensive-guard house style; either way it is provably dead today.)
Suggested fix: delete the check and its five-line comment — the isTerritoryFanOut refusal above already covers every plan the roster can produce. If defense-in-depth for future roster changes is genuinely wanted, derive the refusal and the roster from one shared predicate so the coupling is structural rather than asserted at a call site.
中文说明
[Suggestion] buildWorkflowArgs 中的 chunk-agent 守卫是不可达的死代码——requiredAgents 只在 lib/roster.ts 同一文件内的 isTerritoryFanOut 分支里产出 role: 'chunk'，而本函数在上面两行已经用同一谓词抛出了。
失败场景：该触发条件无法通过公开 API 构造：requiredAgents 唯一的 chunk 产出点受 isTerritoryFanOut(plan) 门控，而本函数已先拒绝它。因此该检查也无法测试（不 mock requiredAgents 就没有 fixture 能触发）。实际代价是读者负担——读者必须追踪 roster.ts 才能发现 "This is a bug in the CLI" 今天永远不可能为真。（3c 和 6c 认为这是刻意的、带注释的未来变更断言，符合本文件防御式守卫风格；无论如何它今天确实不可达。）
建议修复：删除该检查及其五行注释——上面的 isTerritoryFanOut 拒绝已覆盖 roster 能产出的所有 plan。若确实想要针对未来 roster 变化的纵深防御，应让拒绝与 roster 派生自同一个共享谓词，使耦合成为结构性的而非在调用点断言。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| * Separated from the command boundary so the refusals and the roster | ||
| * derivation are testable without a filesystem round trip. | ||
| */ |
There was a problem hiding this comment.
[Suggestion] buildWorkflowArgs is documented as a pure "build the payload" function but performs delivery-critical filesystem writes (buildLaunch writes each brief; the map calls recordPrompt per agent), and runEmitWorkflow performs those writes before validating/creating --out. Probe-verified: with --out pointing at an existing file, the handler throws a raw EEXIST naming neither emit-workflow nor --out, with 13 prompt records already on disk.
Failure scenario: a user passes an unusable --out (an existing file, or an unwritable path) — mkdirSync throws after all briefs and records for the whole roster are already on disk, leaving a record set for a workflow that was never emitted, with a raw fs error that hides where the records went. Separately, any second caller of this exported "build" function that wants only the payload (a dry-run — a plausible next consumer) silently writes prompt records the delivery gate reads as obligations, failing check-coverage with the cause invisible in the calling code. The docblock's "testable without a filesystem round trip" is also literally false — the tests must create temp dirs because the function writes.
Suggested fix: move mkdirSync(outDir) / writeability validation ahead of buildWorkflowArgs, and correct the docblock to state the function writes each agent's brief and records its prompt under the plan's record dir (or move recordPrompt out to runEmitWorkflow to match runRoster's structure).
中文说明
[Suggestion] buildWorkflowArgs 被注释为纯"构建 payload"函数，但实际执行了交付关键的文件系统写入（buildLaunch 写每个 brief；map 中为每个 agent 调用 recordPrompt），且 runEmitWorkflow 在验证/创建 --out 之前就完成了这些写入。探针已验证：--out 指向已存在文件时，handler 抛出裸 EEXIST，既不提 emit-workflow 也不提 --out，且 13 条 prompt 记录已在磁盘上。
失败场景：用户传入不可用的 --out（已存在文件或不可写路径）——mkdirSync 在整份 roster 的 brief 与记录已落盘后抛错，留下从未发出的 workflow 的记录集，且裸 fs 错误看不出记录写到了哪里。另外，任何想要纯 payload 的第二个调用者（dry-run——很可能的后续消费者）会静默写入交付门禁当作义务读取的 prompt 记录，导致 check-coverage 失败而调用代码里看不出原因。docblock 声称的"无需文件系统往返即可测试"也不成立——正因为函数会写入，测试必须创建临时目录。
建议修复：把 mkdirSync(outDir)/可写性校验移到 buildWorkflowArgs 之前，并修正 docblock 说明该函数会写入每个 agent 的 brief 并把 prompt 记录到 plan 的 record 目录（或把 recordPrompt 移到 runEmitWorkflow，对齐 runRoster 的结构）。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| if (!args || !Array.isArray(args.agents)) { | ||
| throw new Error('review-step-3a: args.agents is missing or not an array - pass the args.json that qwen review emit-workflow wrote'); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The script's fail-closed validation covers missing/mis-bound/wrong-version args but not an empty agents array, which yields a clean zero-agent "complete" result. Probe-verified by executing the real constant: {version: 1, agents: []} passes both guards, parallel([]) resolves to [], and the script returns {rosterSize: 0, delivered: [], missingRoles: []} — byte-identical in shape to a fully-complete review with zero agents run.
Failure scenario: a corrupted or hand-edited args.json containing {"version":1,"agents":[]} (the exact shape a truncated/partial write or a future emitter bug could produce — a 3A review always requires multiple agents) passes both guards. Any consumer of the workflow return that treats "no missing roles" as "all dimensions reviewed" would certify a review in which no agent ran. The test enumerates bad shapes [undefined, path-string, {version: 1}] but not the empty-array shape.
Suggested fix: after the array check, add if (args.agents.length === 0) throw new Error('review-step-3a: args.agents is empty - ...') (or fold into the existing condition).
中文说明
[Suggestion] 脚本的 fail-closed 校验覆盖了缺失/错误绑定/版本不符的 args，但没有覆盖空的 agents 数组，后者会产出干净的零 agent "完整"结果。探针已验证（执行真实常量）：{version: 1, agents: []} 通过两道守卫，parallel([]) 解析为 []，脚本返回 {rosterSize: 0, delivered: [], missingRoles: []}——与零 agent 运行、完全完成的评审形状逐字节相同。
失败场景：被损坏或手工编辑的 args.json 含 {"version":1,"agents":[]}（截断/部分写入或未来 emitter bug 可能产生的确切形状——3A 评审必然需要多个 agent）即可通过两道守卫。任何把"无 missingRoles"当作"所有维度均已评审"的消费者都会为一次没有运行任何 agent 的评审背书。测试枚举了坏形状 [undefined, path-string, {version: 1}]，但没有空数组形状。
建议修复：数组检查后加上 if (args.agents.length === 0) throw new Error('review-step-3a: args.agents is empty - ...')（或并入现有条件）。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| phase('Review'); | ||
|
|
||
| // Fail closed on a payload this script cannot dispatch: a missing, stale or |
There was a problem hiding this comment.
[Suggestion] The script's fail-closed guards run after its only other observable side effect — the phase('Review') transition — so an undispatchable payload advances the runtime's phase state before the script throws. Probe-verified: with corrupt args ({version: 1}, no agents), the shipped script records phases=["Review"] before throwing; with the guards moved above phase('Review'), the same input throws with phases=[].
Failure scenario: the same corruption the guards exist to catch (a truncated/hand-edited args.json, or a version the script does not read) advances the run's phase/progress state before failing — the error record and the registry's phaseStarted emission both show a Review phase that never dispatched anything. On a resume-replay runtime — the determinism property this script's header is explicitly built around — the replay re-enters the same started phase against the same corrupt args. Nothing requires the current order.
Suggested fix: move both guard blocks (the args.agents array check and the args.version check) above phase('Review');.
中文说明
[Suggestion] 脚本的 fail-closed 守卫运行在其唯一可观察副作用——phase('Review') 状态转换——之后，因此一个不可派发的 payload 会在脚本抛错前先推进运行时的 phase 状态。探针已验证：对损坏的 args（{version: 1}，无 agents），现脚本先记录 phases=["Review"] 再抛错；把守卫移到 phase('Review') 之前后，同一输入抛错时 phases=[]。
失败场景：守卫要拦截的同一类损坏（截断/手工编辑的 args.json，或脚本不认识的 version）会先推进运行的 phase/进度状态再失败——错误记录和注册表的 phaseStarted 事件都显示一个从未派发任何东西的 Review phase。在 resume-replay 运行时（本脚本头部明确为之构建的确定性属性）上，重放会带着同样的损坏 args 再次进入同一个已开始的 phase。当前顺序没有任何理由要求。
建议修复：把两个守卫块（args.agents 数组检查和 args.version 检查）移到 phase('Review'); 之上。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const plan = report as RosterPlan; | ||
|
|
||
| // 3B is not a bigger 3A. Its chunk agents carry a per-territory contract — |
There was a problem hiding this comment.
[Suggestion] runEmitWorkflow validates only that the plan file parses; a plan file containing JSON null (or any primitive) passes the parse and then dies in roster.ts internals with a raw uncaught TypeError instead of the command's graceful refusal. Probe-verified: a plan file containing null throws TypeError: Cannot read properties of null (reading 'srcDiffLines') at isTerritoryFanOut (roster.ts:108), with no emit-workflow: attribution — unlike every other refusal this command produces. (agent-prompt --roster crashes identically on the same input, so this is new code copying a pre-existing weakness — but the crash is newly reachable through this command's own read-validate-build chain.)
Failure scenario: a corrupted or hand-edited plan.json that happens to contain null — readFileSync reads fine, JSON.parse returns null, and isTerritoryFanOut(plan) dereferences plan.srcDiffLines on the null, producing a bare TypeError with a stack pointing at roster.ts, indistinguishable from a CLI bug in a pipeline log.
Suggested fix: after JSON.parse, validate the shape — if (typeof report !== 'object' || report === null || Array.isArray(report)) throw new Error('emit-workflow: the plan file ... is not a plan object') — or fold the check into a shared plan reader (the extraction f-3a proposes).
中文说明
[Suggestion] runEmitWorkflow 只校验 plan 文件能否解析；包含 JSON null（或任何基本类型）的 plan 文件通过解析后在 roster.ts 内部以裸的未捕获 TypeError 崩溃，而不是命令的优雅拒绝。探针已验证：内容为 null 的 plan 文件在 isTerritoryFanOut（roster.ts:108）抛出 TypeError: Cannot read properties of null (reading 'srcDiffLines')，没有任何 emit-workflow: 前缀——与本命令的其他所有拒绝都不同。（agent-prompt --roster 对同一输入同样崩溃，所以这是新代码复制了一个既有弱点——但该崩溃通过本命令自己的 read-validate-build 链变得新可达。）
失败场景：恰好包含 null 的损坏/手工编辑 plan.json——readFileSync 正常读取，JSON.parse 返回 null，isTerritoryFanOut(plan) 对 null 解引用 plan.srcDiffLines，产生指向 roster.ts 的裸 TypeError，在流水线日志中与 CLI bug 无法区分。
建议修复：JSON.parse 之后校验形状——if (typeof report !== 'object' || report === null || Array.isArray(report)) throw new Error('emit-workflow: the plan file ... is not a plan object')——或把检查并入共享的 plan reader（f-3a 提议的提取）。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const agents = buildRosterLaunches(report, planPath, rules).map( | ||
| ({ req, key, prompt }): WorkflowAgentSpec => { |
There was a problem hiding this comment.
[Suggestion] runEmitWorkflow validates only that the plan parses; a plan file that is a valid JSON object but not a plan (e.g. {}, a settings file, an old plan format) passes all three refusals and dies deep in agent-prompt.ts internals with an agent-prompt:-attributed error — or, for a partial plan, silently emits a full roster from a non-plan. Probe-verified: {} produces 11 agents and throws agent-prompt: the plan has no diffPathAbsolute; a truncated plan (diffPathAbsolute + chunks, no files/budget) completes, writing 11 briefs + 11 prompt records + args.json and printing "11 agents required". Sibling of the null case (ra4-2) whose typeof check does not catch {}.
Failure scenario: qwen review emit-workflow --plan <file> where <file> parses but is not a plan. The {} case dies with a misattributed agent-prompt: error indistinguishable from a genuine agent-prompt failure in a pipeline log. The truncated-plan variant silently emits a full roster from a non-plan: a later check-coverage reads those records as obligations — a review emitted from a non-plan with no error at all. Neither case has a test (the suite covers only unreadable-file, territory, and worktree refusals).
Suggested fix: after JSON.parse, validate plan shape — diffPathAbsolute is a non-empty string, chunks is a non-empty array, files is an array — and throw emit-workflow: --plan <path> is not a plan report; alternatively wrap the buildWorkflowArgs call in runEmitWorkflow so build-chain errors are re-attributed to emit-workflow (same remedy f-6c1 asks for at the writeBrief site, extended to the requireDiffPath/diffReadingBlock sites this command newly reaches).
中文说明
[Suggestion] runEmitWorkflow 只校验 plan 能否解析；是合法 JSON 对象但不是 plan 的文件（如 {}、设置文件、旧格式 plan）能通过全部三道拒绝，然后在 agent-prompt.ts 深处以 agent-prompt: 前缀的错误死亡——而对于部分 plan，则会静默地从非 plan 发出一整份 roster。探针已验证：{} 产出 11 个 agent 并抛 agent-prompt: the plan has no diffPathAbsolute；截断的 plan（有 diffPathAbsolute + chunks，缺 files/budget）则正常完成，写出 11 份 brief + 11 条 prompt 记录 + args.json 并打印 "11 agents required"。这是 null 情形（ra4-2）的兄弟问题，ra4-2 的 typeof 检查拦不住 {}。
失败场景：qwen review emit-workflow --plan <file> 且 <file> 可解析但不是 plan。{} 情形以错误归属的 agent-prompt: 错误死亡，与流水线日志中真正的 agent-prompt 失败无法区分。截断 plan 变体则静默地从非 plan 发出一整份 roster：后续 check-coverage 会把这些记录当作义务读取——一次从非 plan 发出的评审，全程无任何报错。两种情形都没有测试（套件只覆盖不可读文件、territory、worktree 三类拒绝）。
建议修复：JSON.parse 之后校验 plan 形状——diffPathAbsolute 是非空字符串、chunks 是非空数组、files 是数组——并抛 emit-workflow: --plan <path> is not a plan report；或者在 runEmitWorkflow 里包裹 buildWorkflowArgs 调用，把构建链错误重新归属到 emit-workflow（f-6c1 在 writeBrief 处要求的同样补救，扩展到本命令新触达的 requireDiffPath/diffReadingBlock 处）。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const returns = await parallel( | ||
| agents.map((a) => () => agent(a.prompt, { label: a.label, phase: 'Review' })), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The script's fail-closed guards validate the container (args.agents is an array, version matches) but not the elements, and a null (or other primitive) element crashes the fan-out with a raw TypeError instead of the named refusal the guards exist to produce. Probe-verified against the real constant: {"version":1,"agents":[{...},{...},null]} passes both guards; under the real sandbox parallel the accounting loop throws Cannot read properties of null (reading 'key') (under the test-harness analogue it dies mid-map with reading 'prompt') — a bare vm TypeError after phase('Review') and after earlier agents have already dispatched.
Failure scenario: a hand-edited or truncated args.json containing {"version":1,"agents":[{...},{...},null]} passes both guards. The thunk for the null element dereferences a.prompt when parallel invokes it: agents before the null element have already started real subagent dispatches (tokens spent, results discarded) and the fan-out dies raw, mid-dispatch, with no attribution — exactly the failure the guard's own comment says it exists to prevent. Distinct from ra3-1 (element missing prompt → silent promptless dispatch): a null element is a crash path.
Suggested fix: after the array/version guards, validate elements before dispatch, e.g. for (const a of agents) if (a === null || typeof a !== 'object' || typeof a.prompt !== 'string' || typeof a.key !== 'string') throw new Error('review-step-3a: args.agents contains a malformed agent entry — re-run qwen review emit-workflow') (this also subsumes ra3-1's prompt check).
中文说明
[Suggestion] 脚本的 fail-closed 守卫只校验容器（args.agents 是数组、version 匹配），不校验元素；一个 null（或其他基本类型）元素会让扇出以裸 TypeError 崩溃，而不是守卫本应产出的具名拒绝。探针已验证（执行真实常量）：{"version":1,"agents":[{...},{...},null]} 通过两道守卫；在真实沙箱 parallel 下，记账循环抛 Cannot read properties of null (reading 'key')（在测试 harness 模拟下则是在 map 中途以 reading 'prompt' 死亡）——在 phase('Review') 之后、且前面的 agent 已经派发之后，出现裸的 vm TypeError。
失败场景：手工编辑或截断的 args.json 含 {"version":1,"agents":[{...},{...},null]} 即可通过两道守卫。parallel 调用时，null 元素的 thunk 对 a.prompt 解引用：null 元素之前的 agent 已开始真实子代理派发（token 已花、结果被丢弃），扇出在中途裸死、无任何归属——正是守卫自己注释声称要防止的失败。与 ra3-1（元素缺 prompt → 静默无 prompt 派发）不同：null 元素是崩溃路径。
建议修复：数组/version 守卫之后，在派发前校验元素，如 for (const a of agents) if (a === null || typeof a !== 'object' || typeof a.prompt !== 'string' || typeof a.key !== 'string') throw new Error('review-step-3a: args.agents contains a malformed agent entry — re-run qwen review emit-workflow')（这也涵盖 ra3-1 的 prompt 检查）。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 2/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31389561905 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no such file or directory.
中文说明
Test Plan（非阻断）：src/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // The delivery gate reads recorded prompts, not briefs: without this | ||
| // record, a run dispatched from these args would fail check-coverage as | ||
| // "briefless" despite reviewing with exactly the built prompts. | ||
| recordPrompt(planPath, key, prompt); |
There was a problem hiding this comment.
[Critical] R2-1: The round-1 fix is only half the delivery gate. recordPrompt populates the register coverageFromTranscripts reads via readRecordedPrompts (lib/coverage.ts), but the gate reads a second register too — subagent transcripts (readTranscripts, <projectDir>/subagents/<sessionId>/agent-*.jsonl) — and workflow dispatch never writes one: runSingleDispatch → AgentHeadless attaches no JSONL transcript writer (the only production callers of attachJsonlTranscriptWriter are the Agent tool and background-agent-resume). This is the still-open sibling entrance of round-1 blocker R1-1 — the briefless entrance is closed, the transcript entrance is not — and it corroborates @wenshao's open thread. — Failure scenario: dispatch the emitted workflow and run check-coverage: every built role lands in missingRoles ("its prompt was built, but no agent on record was launched with it") — or TranscriptsUnavailableError in a fresh session — and the gate exits 3, so a fully correct run is reported to the PR author as entirely unreviewed. Probe-verified at this commit with a flip arm: fabricating one Agent-tool-style JSONL per recorded role clears missingRoles; without it every role fails. The new recorded-prompt-parity assertion passes while the property it exists to protect does not hold. Suggested fix: do not claim gate acceptance via prompt records on the workflow path — either sequence per #8769 (land journal-based coverage before routing anything through this artifact, and adjust the comment/test framing meanwhile), or make workflow dispatch attach the same JSONL transcript writer the Agent tool uses.
中文说明
第一轮修复只覆盖了交付门禁的一半。recordPrompt 填充了 coverageFromTranscripts 通过 readRecordedPrompts（lib/coverage.ts）读取的登记表，但门禁还读取第二个登记表——子代理 transcript（readTranscripts，<projectDir>/subagents/<sessionId>/agent-*.jsonl）——而 workflow 分发从不写入它：runSingleDispatch → AgentHeadless 没有挂载任何 JSONL transcript writer（attachJsonlTranscriptWriter 仅有的生产调用方是 Agent 工具与 background-agent-resume）。这是第一轮 blocker R1-1 仍然敞开的同族入口——briefless 入口已关闭，transcript 入口没有——并与 @wenshao 的开放线程相互印证。失败场景：分发该 workflow 并运行 check-coverage：每个已构建角色落入 missingRoles（"its prompt was built, but no agent on record was launched with it"）——新会话则直接 TranscriptsUnavailableError——门禁以 exit 3 结束，一次完全正确的运行被报告为"完全未评审"。已在本提交上用探针加翻转臂验证：为每个已记录角色伪造一条 Agent 工具风格的 JSONL 后 missingRoles 清空；不伪造则全部失败。新增的"记录 prompt 逐字节一致"断言通过，但它所要保护的性质并不成立。建议修复：不要在 workflow 路径上用 prompt 记录声称门禁通过——要么按 #8769 排序（先落地基于 journal 的 coverage 再接线，期间修正注释/测试表述），要么让 workflow 分发挂载与 Agent 工具相同的 JSONL transcript writer。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // mis-bound args file would otherwise die inside the vm as a bare TypeError | ||
| // that names nothing about emit-workflow or the args file. | ||
| if (!args || !Array.isArray(args.agents)) { | ||
| throw new Error('review-step-3a: args.agents is missing or not an array - pass the args.json that qwen review emit-workflow wrote'); |
There was a problem hiding this comment.
[Suggestion] R2-2: The guard's remediation hint is circular for the most likely misuse: passing the args file's path — exactly what emit-workflow's stdout instructs ("make one Workflow call with the paths below", args: <path>) — IS passing "the args.json that qwen review emit-workflow wrote". The actual remediation is never named: read the file and pass its PARSED JSON CONTENTS inline, because the vm sandbox has no filesystem and Workflow accepts args inline-only ("Pass actual JSON, not a stringified value"). — Concrete cost: an orchestrator that follows the stdout line passes the path string, is told to pass what it just passed, reads the demand as unsatisfiable, and retries the identical call; the real cause is nowhere in the error. The PR's own test pins the path-string case to this same hint.
| throw new Error('review-step-3a: args.agents is missing or not an array - pass the args.json that qwen review emit-workflow wrote'); | |
| throw new Error('review-step-3a: args.agents is missing or not an array - args must be the PARSED CONTENTS of the args.json that qwen review emit-workflow wrote (pass the JSON value, not the path; the sandbox cannot read files)'); |
中文说明
守卫的补救提示对最可能的误用是循环的：传入 args 文件的路径——这正是 emit-workflow 的 stdout 所指示的（"make one Workflow call with the paths below"、args: <path>）——本身就是"传入 emit-workflow 写出的 args.json"。真正的补救方式从未被点名：读取文件并把其解析后的 JSON 内容以内联值传入，因为 vm 沙箱没有文件系统，Workflow 的 args 只接受内联 JSON（"Pass actual JSON, not a stringified value"）。具体代价：orchestrator 照 stdout 传入路径字符串，却被要求"传入你刚刚传入的东西"，会认为该要求无法满足，从而原样重试同一个调用；错误的真实原因在报错里完全看不到。PR 自己的测试也把路径字符串这一用例钉在了同一个提示上。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const emitted = payload.agents.find((a) => a.key === req.key); | ||
| expect(emitted?.label).toBe(rosterLabel(req)); |
There was a problem hiding this comment.
[Suggestion] R2-4: rosterLabel's output is pinned only by a circular oracle — this assertion compares each emitted label to rosterLabel(req) itself, so it verifies the wiring (emit-workflow uses rosterLabel) but not the output: both could be wrong together. No literal expected strings exist anywhere for the chunk case, the role case, or the control-character flattening. Test-efficacy probe (harness validated): reverting the rosterLabel export hunk on its own left the colocated agent-prompt suite green; this tautological cross-file assertion is the only gate. Residual of round-1 R1-13. — Failure scenario: a regression corrupting rosterLabel output (a chunk labelled as a role, a separator glyph leaking through the flattening) ships green and mislabels every emitted workflow step in the runtime's progress display. Suggested fix: add direct unit tests for rosterLabel with literal expected strings (chunk case, role case, control-character flattening, separator stripping), independent of the emitted payload.
中文说明
rosterLabel 的输出只被一个循环 oracle 钉住——该断言把每个产出的 label 与 rosterLabel(req) 本身比较，因此只验证了接线（emit-workflow 使用 rosterLabel），没有验证输出：两者可以同时是错的。chunk 分支、role 分支、控制字符扁平化都不存在任何字面期望字符串。测试效力探针（harness 已验证）：单独还原 rosterLabel 的导出 hunk 后，同目录的 agent-prompt 测试套件仍全绿；这个同义反复的跨文件断言是唯一的把关。这是第一轮 R1-13 的残留。失败场景：一个破坏 rosterLabel 输出的回归（把 chunk 标成 role、分隔符图形从扁平化中泄漏）会全绿通过，并在运行时进度展示中错标每个 workflow 步骤。建议修复：为 rosterLabel 增加带字面期望字符串的直接单元测试（chunk、role、控制字符扁平化、分隔符剥离），独立于产出的 payload。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 3/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31394772225 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. 3 of those were silent-sandbox (idle) timeouts that no budget increase can cure — investigate the sandbox image and runner docker daemon for those. A human should investigate the sandbox image and runner docker daemon, then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31397491013 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
|
@qwen-code /takeover stop |
qwen-code-dev-bot
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no 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/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no such file or directory。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| recordPrompt(planPath, key, prompt); | ||
| return { key, label: rosterLabel(req), prompt }; |
There was a problem hiding this comment.
[Critical] R2-1: The round-1 fix is only half the delivery gate. recordPrompt populates the register coverageFromTranscripts reads via readRecordedPrompts, but the gate reads a second register too — subagent transcripts (readTranscripts, <projectDir>/subagents/<sessionId>/agent-*.jsonl) — and workflow dispatch never writes one: attachJsonlTranscriptWriter is called only by the Agent tool and background-agent-resume.
Failure scenario: dispatch the emitted workflow and run check-coverage: every built role lands in missingRoles ("its prompt was built, but no agent on record was launched with it") — or TranscriptsUnavailableError in a fresh session — and the gate exits 3, so a fully correct run is reported to the PR author as entirely unreviewed.
Suggested fix: do not claim gate acceptance via prompt records on the workflow path — either sequence per #8769 (land journal-based coverage before routing anything through this artifact, and adjust the comment/test framing meanwhile), or make workflow dispatch attach the same JSONL transcript writer the Agent tool uses.
中文说明
第一轮修复只覆盖了交付门禁的一半。recordPrompt 填充了 coverageFromTranscripts 通过 readRecordedPrompts 读取的登记表，但门禁还读取第二个登记表——子代理 transcript（readTranscripts，<projectDir>/subagents/<sessionId>/agent-*.jsonl）——而 workflow 分发从不写入它：attachJsonlTranscriptWriter 仅有的生产调用方是 Agent 工具与 background-agent-resume。
失败场景：分发该 workflow 并运行 check-coverage：每个已构建角色落入 missingRoles（"its prompt was built, but no agent on record was launched with it"）——新会话则直接 TranscriptsUnavailableError——门禁以 exit 3 结束，一次完全正确的运行被报告为"完全未评审"。
建议修复：不要在 workflow 路径上用 prompt 记录声称门禁通过——要么按 #8769 排序（先落地基于 journal 的 coverage 再接线，期间修正注释/测试表述），要么让 workflow 分发挂载与 Agent 工具相同的 JSONL transcript writer。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| let rules: string | undefined; | ||
| if (args.rules) { | ||
| try { | ||
| rules = readFileSync(args.rules, 'utf8'); |
There was a problem hiding this comment.
[Suggestion] R1-1: runEmitWorkflow adds a 6th copy of the read-plan-or-throw block and a 2nd read-rules-or-throw block, duplicating agent-prompt.ts — and the copy has already drifted: the agent-prompt rules error ends with "...would silently review without the project rules it was told to enforce" while this copy ends at "Omit --rules if this review has none."
Failure scenario: a future fix to one copy (e.g. distinguishing ENOENT from JSON parse failure) will not reach the others; a run under emit-workflow with a bad --rules path gets the shorter, less informative refusal.
Suggested fix: extract the pair into lib/ — e.g. readPlanFile(path, cmd) and readRulesFile(path, cmd) — and call them from both agent-prompt.ts and emit-workflow.ts.
中文说明
runEmitWorkflow 新增了第 6 份 read-plan-or-throw 代码块和第 2 份 read-rules-or-throw 代码块，与 agent-prompt.ts 重复——而且这份拷贝已经开始漂移：agent-prompt 的 rules 报错以"...would silently review without the project rules it was told to enforce"结尾，而这份拷贝只到"Omit --rules if this review has none."为止。
失败场景：未来对某一处的修复（例如区分 ENOENT 与 JSON 解析失败）不会到达其他拷贝；在 emit-workflow 下以错误的 --rules 路径运行，用户只会得到更短、信息更少的拒绝信息。
建议修复：把这一对抽取到 lib/——例如 readPlanFile(path, cmd) 和 readRulesFile(path, cmd)——并让 agent-prompt.ts 与 emit-workflow.ts 共同调用。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const chunk = requiredAgents(plan).find((r) => r.role === 'chunk'); | ||
| if (chunk) { |
There was a problem hiding this comment.
[Suggestion] R1-3: The chunk-agent guard in buildWorkflowArgs is unreachable dead code — requiredAgents emits role: 'chunk' only inside its own isTerritoryFanOut branch (lib/roster.ts), the exact predicate the territory refusal ~20 lines above already throws on; the guard also computes the full roster a second time per emit.
Failure scenario: no runtime trigger exists — the branch can never fire; the cost is a duplicated requiredAgents walk per invocation and a branch implying a failure mode the preceding gates make impossible.
Suggested fix: delete the guard (the territory refusal is the real gate), or move the assert into buildRosterLaunches where both dispatchers pass through.
中文说明
buildWorkflowArgs 中的 chunk-agent 守卫是不可达的死代码——requiredAgents 只在 lib/roster.ts 自身的 isTerritoryFanOut 分支内产出 role: 'chunk'，而这正是上方约 20 行处 territory 拒绝已经抛错的同一谓词；该守卫还让每次 emit 额外完整计算一次 roster。
失败场景：不存在运行时触发条件——该分支永远不会执行；代价是每次调用重复计算 requiredAgents，以及一个暗示着前面两道门禁已经排除的失败模式的分支。
建议修复：删除该守卫（territory 拒绝才是真正的门禁），或把断言移到两个分发器都经过的 buildRosterLaunches 中。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| export function buildWorkflowArgs( | ||
| report: PlanReport, | ||
| planPath: string, | ||
| rules?: string, | ||
| ): WorkflowArgsFile { |
There was a problem hiding this comment.
[Suggestion] R1-4: buildWorkflowArgs is documented as a pure "build the payload" function but performs delivery-critical filesystem writes (buildRosterLaunches → buildLaunch → writeBrief; recordPrompt per agent) before the --out write is ever validated. Probe-verified: with --out naming an existing file, the run throws EEXIST after 13 briefs and 13 prompt records are already on disk.
Failure scenario: a run that never dispatched and never will has populated the delivery gate's obligation register (readRecordedPrompts), and nothing distinguishes an aborted emit's records from a completed build's.
Suggested fix: move recordPrompt (and document the brief writes) to the command boundary after the --out write succeeds, or correct the doc comment to state the builder commits briefs/records.
中文说明
buildWorkflowArgs 的文档声称它是纯粹的"构建 payload"函数，但它会在 --out 写入被验证之前执行交付关键的文件系统写入（buildRosterLaunches → buildLaunch → writeBrief；每个 agent 的 recordPrompt）。已用探针验证：当 --out 指向已存在的文件时，运行在 13 份 brief 与 13 条 prompt 记录落盘之后才抛出 EEXIST。
失败场景：一次从未分发、也永远不会分发的运行已经填充了交付门禁的义务登记表（readRecordedPrompts），而中止的 emit 记录与完成的构建记录没有任何区别。
建议修复：把 recordPrompt（并在注释中说明 brief 写入）移到 --out 写入成功之后的命令边界，或修正文档注释以说明该 builder 会提交 brief/记录。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| if (args.version !== 1) { | ||
| throw new Error('review-step-3a: args version ' + args.version + ' does not match this script - re-run qwen review emit-workflow to regenerate both files together'); | ||
| } | ||
| const agents = args.agents; |
There was a problem hiding this comment.
[Suggestion] R1-5: The script's fail-closed validation covers missing/mis-bound/wrong-version args but not an empty agents array, which yields a clean zero-agent "complete" result. Probe-verified by executing the real constant: {version: 1, agents: []} returns {rosterSize: 0, delivered: [], missingRoles: []} — no error.
Failure scenario: a hand-written or truncated args.json with an empty agents array produces a "completed review that reviewed nothing", indistinguishable from a clean run.
Suggested fix: reject agents.length === 0 in the same guard block (throw the same emit-workflow-attributed message).
中文说明
脚本的 fail-closed 校验覆盖了缺失/绑定错误/版本不符的 args，但没有覆盖空的 agents 数组——空数组会得到干净的零 agent "完成"结果。已通过执行真实常量用探针验证：{version: 1, agents: []} 返回 {rosterSize: 0, delivered: [], missingRoles: []}——没有任何报错。
失败场景：手写或被截断的 args.json 携带空 agents 数组时，产生"评审了但什么也没评审"的结果，与干净运行无法区分。
建议修复：在同一守卫块中拒绝 agents.length === 0（抛出同样的 emit-workflow 归因错误信息）。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| ); | ||
| } | ||
|
|
||
| const agents = buildRosterLaunches(report, planPath, rules).map( |
There was a problem hiding this comment.
[Suggestion] R1-8: runEmitWorkflow validates only that the plan parses; a valid-JSON non-plan object (e.g. {}, a settings file, an old plan format) passes all three refusals and dies deep in agent-prompt internals with an agent-prompt:-attributed error, or silently emits a full roster for a plan that never was one. Probe-verified: {} throws agent-prompt: the plan has no \diffPathAbsolute`...` — pointing the user at the wrong command.
Failure scenario: a caller feeds a non-plan JSON object; the command fails inside buildRosterLaunches with an error naming the wrong command, or emits a roster derived from garbage fields.
Suggested fix: validate plan shape (diffPathAbsolute, chunks[]) up front in runEmitWorkflow with an emit-workflow:-prefixed error, as check-coverage's readPlan already does.
中文说明
runEmitWorkflow 只校验 plan 文件能解析；一个合法 JSON 但非 plan 的对象（例如 {}、settings 文件、旧版 plan 格式）会通过全部三道拒绝，然后死在 agent-prompt 内部深处，抛出 agent-prompt: 归因的错误，或为一份根本不是 plan 的文件静默发出完整 roster。已用探针验证：{} 抛出 agent-prompt: the plan has no \diffPathAbsolute`...`——把用户指向了错误的命令。
失败场景：调用者传入非 plan 的 JSON 对象；命令在 buildRosterLaunches 内部失败，错误指向错误的命令，或基于垃圾字段发出 roster。
建议修复：在 runEmitWorkflow 中提前校验 plan 形状（diffPathAbsolute、chunks[]），抛出 emit-workflow: 前缀的错误——正如 check-coverage 的 readPlan 所做。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const outDir = resolve(args.out); | ||
| mkdirSync(outDir, { recursive: true }); |
There was a problem hiding this comment.
[Suggestion] R2-2: The --out write path silently overwrites existing script.js/args.json and throws raw unwrapped Node errors (EEXIST/EACCES) on failure — the only failure path in the command without an emit-workflow: diagnostic and without a test. Probe-verified: pre-existing user files with those names are clobbered with no existence check; --out naming an existing file throws EEXIST: file already exists, mkdir '<path>'.
Failure scenario: run qwen review emit-workflow --plan p.json --out . in a populated directory — an existing script.js (a common build-script name) or args.json is silently destroyed; or --out names an existing file and the raw EEXIST error names neither --out nor that a directory is required.
Suggested fix: fail closed when either target already exists (or require an empty/newly-created out dir), and wrap the mkdir/write block in a try/catch rethrowing emit-workflow: cannot write into --out <path>: <message>.
中文说明
--out 写入路径会静默覆盖已存在的 script.js/args.json，并在失败时抛出原始未包装的 Node 错误（EEXIST/EACCES）——这是该命令中唯一没有 emit-workflow: 诊断、也没有测试的失败路径。已用探针验证：同名用户文件在没有存在性检查的情况下被覆盖；--out 指向已存在的文件时抛出 EEXIST: file already exists, mkdir '<path>'。
失败场景：在已有内容的目录中运行 qwen review emit-workflow --plan p.json --out .——已存在的 script.js（常见的构建脚本名）或 args.json 被静默销毁；或者 --out 指向一个已存在的文件，原始 EEXIST 错误既不提及 --out 也不说明需要目录。
建议修复：当任一目标已存在时 fail closed（或要求 out 目录为空/新建），并用 try/catch 包装 mkdir/write 块，重抛为 emit-workflow: cannot write into --out <path>: <message>。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| if (key !== req.key) { | ||
| throw new Error( | ||
| `built "${key}" where the roster requires "${req.key}" — the ` + |
There was a problem hiding this comment.
[Suggestion] R3-2: Extracting the key-mismatch guard into the shared buildRosterLaunches silently dropped the agent-prompt: --roster prefix the --roster path's error carried before this diff — the mismatch error is now the one un-attributed diagnostic on the path that produced it, and no test pins either message.
Failure scenario: run qwen review agent-prompt --plan plan.json --roster on a plan where the roster key derivation and buildLaunch's key disagree (the exact internal-CLI-bug condition the guard exists for): the user gets an unprefixed error while every other error from the same build path keeps its agent-prompt: prefix.
Suggested fix: restore command attribution in the shared message, e.g. `agent-prompt: built "${key}" where the roster requires "${req.key}" — ...` — or parametrize the prefix so --roster and emit-workflow each get their own.
中文说明
把 key 不一致守卫抽取进共享的 buildRosterLaunches 时，静默丢失了 --roster 路径在本 diff 之前错误信息所带的 agent-prompt: --roster 前缀——这条不匹配错误现在成为该构建路径上唯一没有命令归属的诊断，且两个信息都无测试固定。
失败场景：在 roster key 派生与 buildLaunch key 不一致的 plan 上运行 qwen review agent-prompt --plan plan.json --roster（这正是该守卫存在的内部 CLI bug 条件）：用户得到无前缀的错误，而同一条构建路径上的其他错误都保留 agent-prompt: 前缀。
建议修复：在共享信息中恢复命令归属，例如 `agent-prompt: built "${key}" where the roster requires "${req.key}" — ...`——或将前缀参数化，让 --roster 与 emit-workflow 各自得到正确归属。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const value = returns[i]; | ||
| if (value === null || value === undefined) { | ||
| missingRoles.push(agents[i].key); |
There was a problem hiding this comment.
[Suggestion] R3-3: The script's fail-closed accounting treats an empty-string return as delivered, not missing — '' is neither null nor undefined, so it lands in delivered with text: '' and the run reports a clean missingRoles: []. Probe-verified: a GOAL termination with empty final text resolves to '' through the real dispatch chain and is counted as delivered.
Failure scenario: a workflow subagent that terminates GOAL with empty visible text (an analysis-only or empty final message) is counted as delivered-with-no-text — {rosterSize: N, delivered: [...], missingRoles: []} — exactly the "completed review that reviewed nothing" shape the script's own comments say it exists to prevent.
Suggested fix: treat empty/whitespace-only returns as missing too: if (value === null || value === undefined || (typeof value === 'string' && value.trim().length === 0)).
中文说明
脚本的 fail-closed 记账把空字符串返回视为 delivered 而非 missing——'' 既不是 null 也不是 undefined，所以它会以 text: '' 进入 delivered，运行报告出干净的 missingRoles: []。已用探针验证：最终文本为空的 GOAL 终止会通过真实分发链解析为 '' 并被计入 delivered。
失败场景：workflow 子代理以空可见文本终止 GOAL（仅分析或空最终消息）时，被记为"已交付但无文本"——{rosterSize: N, delivered: [...], missingRoles: []}——正是脚本自身注释声称要防止的"评审了但什么也没评审"形态。
建议修复：把空/纯空白返回也视为 missing：if (value === null || value === undefined || (typeof value === 'string' && value.trim().length === 0))。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
| const body = REVIEW_STEP_3A_WORKFLOW_SCRIPT.replace( | ||
| 'export const meta =', | ||
| 'const meta =', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R4-1: The script is executed for testing only in the Node host realm via new AsyncFunction — never under the real createWorkflowSandbox vm, which differs in three named ways: it runs the body as (async () => {...})(), on a null-prototype globalThis with no process/require, and it removes the entire meta declaration (stripExportMeta) where the harness merely rewrites export const meta = to const meta =. Probe-verified: a body using process.env.X or referencing meta passes all 9 tests and throws inside the vm.
Failure scenario: a future edit adding a host-only global (process.env.X) or referencing meta passes all 9 tests and throws ReferenceError at workflow dispatch, where no unit test sees it.
Suggested fix: execute the constant through the real sandbox (export createWorkflowSandbox or add a minimal runWorkflowScript helper), or add a host-realm isolation test running the body in a fresh vm.createContext(Object.create(null)) with only the documented globals injected.
中文说明
脚本仅通过 new AsyncFunction 在 Node 宿主 realm 中执行测试——从未在真实的 createWorkflowSandbox vm 下运行，后者有三处不同：以 (async () => {...})() 包裹执行、在无 process/require 的 null-prototype globalThis 上运行、并且会整体移除 meta 声明（stripExportMeta），而测试 harness 只是把 export const meta = 改写为 const meta =。已用探针验证：使用 process.env.X 或引用 meta 的脚本体通过全部 9 条测试，但在 vm 内抛错。
失败场景：未来加入宿主专属全局（process.env.X）或引用 meta 的编辑通过全部 9 条测试，却在 workflow 分发时抛出 ReferenceError——任何单元测试都看不到。
建议修复：通过真实 sandbox 执行该常量（导出 createWorkflowSandbox 或添加最小 runWorkflowScript 助手），或增加宿主隔离测试，在只注入文档化全局的全新 vm.createContext(Object.create(null)) 中运行脚本体。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: integration tests (CLI, no sandbox) — the applicable GitHub check was skipped and Agent 7's scoped build-test run did not execute this integration configuration.
Test Plan (not a blocker): src/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no such file or directory.
中文说明
未审查：integration tests (CLI, no sandbox) — the applicable GitHub check was skipped and Agent 7's scoped build-test run did not execute this integration configuration。
Test Plan（非阻断）：src/commands/review.test.ts — no such file or directory; src/commands/review/emit-workflow.test.ts — no such file or directory; src/commands/review/workflow-script.test.ts — no such file or directory。
— gpt-5.6-sol via Qwen Code /review (v0.21.9)
| if (!args || !Array.isArray(args.agents)) { | ||
| throw new Error('review-step-3a: args.agents is missing or not an array - pass the args.json that qwen review emit-workflow wrote'); | ||
| } | ||
| if (args.version !== 1) { |
There was a problem hiding this comment.
[Critical] R2-2: The workflow trusts any caller-supplied version-1 agents array instead of binding it to the CLI-emitted roster. — Failure scenario: an orchestrator omits Agent 2 or rewrites a prompt in a version-1 payload; the script dispatches and reports success from that forged array, defeating the feature's roster-width and prompt-fidelity guarantee. Bind the complete payload to immutable CLI-produced provenance, such as a digest encoded into the emitted artifact.
中文说明
Workflow 信任任意由调用方提供的 version-1 agents 数组，而没有把它绑定到 CLI 发出的 roster。失败场景：orchestrator 在 version-1 payload 中删掉 Agent 2 或改写某个 prompt；脚本仍会分发该伪造数组，并以同一数组报告成功，从而破坏本功能要保证的 roster 宽度与 prompt 保真。请把完整 payload 绑定到不可变的 CLI 产物溯源，例如在发出的 artifact 中编码 digest。
— gpt-5.6-sol via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
Declined — the proposed binding cannot hold under this trust model. For a digest to detect a forged agents array, the verifier of the digest must be outside the forger's control. Here both halves travel together in the same model-authored Workflow call: args is inline-only (verified in WORKFLOW_PARAM_SCHEMA), so a caller that rewrites the array rewrites any digest beside it. Baking the digest into the emitted script does not help either — the Workflow tool accepts fully model-authored inline script (the XOR with scriptPath), so the script itself is not an immutable anchor the model cannot bypass.
The design's enforcement point is the delivery gate, not the script: check-coverage pairs harness-written evidence against CLI-written records — a dropped Agent 2 lands in missingRoles, a rewritten prompt lands in rewrittenPrompts. That is exactly the reported failure scenario, detected after the fact with evidence the orchestrator cannot retcon. The one genuine gap in that story — workflow dispatch writing no transcripts — is the R2-1 cluster (sequenced per #8769, comments/tests corrected this round). Adding a digest field the script cannot actually verify would be a dead switch per Simplicity First; happy to revisit if a runtime-side binding (e.g. the journal paired against the records) is wanted as a follow-up.
中文说明
拒绝——在该信任模型下所提议的绑定无法成立。 digest 要能检测伪造的 agents 数组，digest 的校验方必须在伪造者的控制之外。而此处两半都经由同一个模型编写的 Workflow 调用传递：args 仅支持内联（已在 WORKFLOW_PARAM_SCHEMA 核实），能改写数组的调用方同样能改写其旁的任何 digest。把 digest 烧进发出的脚本也无济于事——Workflow 工具同样接受完全由模型编写的内联 script（与 scriptPath 互斥），脚本本身并不是模型绕不过去的不可篡改锚点。
本设计的强制点是交付门禁，而非脚本：check-coverage 把 harness 写入的证据与 CLI 写入的记录配对——被丢弃的 Agent 2 落入 missingRoles，被改写的 prompt 落入 rewrittenPrompts。这恰好覆盖所报告的失败场景，且是事后以 orchestrator 无法篡改的证据检出。该链条上真正的缺口——workflow 分发不写 transcript——即 R2-1 聚类（已按 #8769 排序，本轮修正了注释/测试表述）。按 Simplicity First，为脚本加一个它实际无法校验的 digest 字段只是死开关；如果希望改为运行时侧绑定（例如 journal 与记录配对），很乐意作为后续工作再议。
| if (value === null || value === undefined) { | ||
| missingRoles.push(agents[i].key); | ||
| } else { | ||
| delivered.push({ key: agents[i].key, text: value }); |
There was a problem hiding this comment.
[Critical] R2-3: An empty model-visible agent result is counted as successfully delivered. — Failure scenario: a GOAL subagent returns analysis-only output that is stripped to ''; this branch adds the role to delivered and omits it from missingRoles, so the review appears complete without a finding or examination receipt. Treat non-string and empty-after-trimming results as missing, and add an empty-string regression test.
中文说明
空的 model-visible agent 结果会被计为成功交付。失败场景：一个 GOAL subagent 只返回 analysis 内容，清洗后变成空字符串 ''；当前分支仍把该角色加入 delivered，且不放入 missingRoles，于是 review 在没有 finding 或检查回执的情况下看起来已完成。请把非字符串和 trim 后为空的结果视为 missing，并添加空字符串回归测试。
— gpt-5.6-sol via Qwen Code /review (v0.21.9)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
… fail closed
Address PR review: the printed handoff described a Workflow call the tool
cannot honor. Write the script into the saved-workflow dir (the only place
`Workflow({scriptPath})` loads from), print an args contract that matches
the tool's inline-only `args`, and refuse up front when workflows are
disabled. Harden the emitted script fail-closed (guard order, payload and
element shape, empty-roster and empty-return accounting, zero-delivery
throw), restore error attribution, drop dead payload fields and the
unreachable chunk guard, validate plan shape, and share one plan/rules
reader between agent-prompt and emit-workflow.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback round — PR #8846 (
|
Superseded by head 7e9c945; no current unreplied Critical remains. Clearing stale CHANGES_REQUESTED state at the PR author request.
|
Superseded by #8943 — closing this one. The review was right and the handoff layer here does not work, so this is a rewrite rather than a patch:
Also addressed there: the missing The claim that this was a one-variable A/B was also wrong, and thanks for catching it: workflow dispatch substitutes its own terse subagent persona, so identical prompts were producing different agents. #8943 passes |
…ript Supersedes QwenLM#8846, whose handoff layer did not work: it wrote the script to an arbitrary `--out` directory and told the caller to pass `args: <path>`. Neither is a form the Workflow tool accepts. `readWorkflowFileSecurely` realpaths `scriptPath` and refuses anything outside the saved-workflow directories, and `args` is inline JSON with no path form. Both were verified against the tool's own contract this time, not just the sandbox's. The sandbox has no filesystem — its globals are agent/parallel/pipeline/ phase/log/console/args/budget/workflow and nothing that opens a file — so a roster carried in `args` is a roster the model has to retype into its tool call. That is the failure this change exists to remove, reintroduced one layer up. The roster is therefore baked into the generated script, and the model's call carries one path and no payload. `--roster` and `emit-workflow` still build the same prompts from the same plan through the same `buildLaunch`. What differs is who launches them: `--roster` asks the orchestrator to copy ~13 blocks into agent calls, in one response, unedited; this writes them into a file the runtime reads. The generated file is a fixed body plus one JSON literal. No logic is generated, so the part that can be wrong is the part the tests execute — and they execute the real output of the generator, including a roster with backticks, `${`, backslashes and newlines in the prompts. Not a one-variable change, and the difference is now handled rather than claimed away: workflow dispatch substitutes its own terse subagent persona unless an agentType is given, so the script passes `agentType: 'general-purpose'` — the subagent type SKILL.md requires of the hand-launched path. Otherwise the two paths would run different agents over identical prompts and an A/B between them would not mean anything. Also from the QwenLM#8846 review: - Prompts are recorded. `check-coverage` compares each launch against what the CLI recorded handing out, so without this the whole roster read as unlaunched. - The workflows gate is checked before anything is written, so a run that cannot execute what this emits leaves no script and no prompt records implying it did. - A fan-out where every agent failed throws instead of returning a value. Returned, it would let the caller aggregate over a diff no agent read. - Cleanup sweeps the generated script. It has to live in the user's saved-workflow dir, where every file is a `/<name>` slash command, so a review that left one behind would hand the user a permanent command for a diff that no longer exists. - The dead `label` / `version` / `mode` / `plan` fields are gone with the args file, and the unreachable chunk guard with them. - `readPlanReport` is one definition in lib/report.ts rather than a sixth copy of the try/parse/rethrow block; moving the existing five onto it is mechanical and left to its own change. Still refused, unchanged: a territory fan-out (3B), whose chunk agents carry a per-chunk contract this script does not express, and a worktree review, which needs every agent pinned to the PR worktree. Draft: this depends on QwenLM#8839 for coverage evidence — workflow-dispatched agents write no subagent transcript without it — and nothing routes through this yet, so review behaviour is unchanged. Part of QwenLM#8769. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 participants
What this PR does
Adds a deterministic
qwen review emit-workflowsubcommand that writes Review Step 3A's complete agent fan-out as a fixed workflow script plus an arguments file. The emitted workflow dispatches the same roster with the same prompts as the existing hand-launched path and reports missing agent returns by roster key.Nothing routes through this command yet. The review skill and finding-delivery format remain unchanged so the code-driven dispatcher can be evaluated independently.
Why it's needed
The current hand-launched path asks an orchestrator to copy every roster entry into separate agent calls without shortening the roster or rewriting prompts. Those conventions can fail after planning has already succeeded. Emitting the fan-out as executable data makes roster width and prompt fidelity properties of the program, while preserving the existing briefs, coverage accounting, and findings flow.
The command fails closed for territory fan-outs and worktree reviews because the Step 3A workflow cannot yet represent those execution contracts safely.
Reviewer Test Plan
How to verify
From
packages/cli, runnpx vitest run src/commands/review.test.ts src/commands/review/emit-workflow.test.ts src/commands/review/workflow-script.test.tsand expect all 22 cases to pass. Then, from the repository root, runnpm run dev -- review emit-workflow --plan <step-1-plan.json> --out <empty-output-directory>with a normal local or diff-only Step 1 plan. Expect the command to print the required agent count and the paths toscript.jsandargs.json; the arguments file should contain every required roster key and its unmodified launch prompt, while stdout should not print the prompts themselves. A territory plan or a PR-worktree plan should stop with a reason and direct the reviewer toagent-prompt --rosterinstead of emitting a misleading workflow.Evidence (Before & After)
N/A — this is an additive internal review helper and is not routed into user-visible review behavior yet.
Tested on
Environment (optional)
Node.js 25.9.0, npm 11.12.1, no sandbox.
Risk & Scope
Linked Issues
Part of #8769.
中文说明
本 PR 做了什么
新增确定性的
qwen review emit-workflow子命令，把 Review Step 3A 的完整 agent 扇出写成固定 workflow 脚本和参数文件。生成的 workflow 使用与现有手工发射路径相同的 roster 和 prompt，并按 roster key 报告没有返回结果的 agent。目前没有任何流程路由到该命令。review skill 和 finding 交付格式保持不变，以便独立评估代码驱动的分派器。
为什么需要
现有手工发射路径要求 orchestrator 把每个 roster 条目复制到独立的 agent 调用中，同时不能缩短 roster 或改写 prompt。这些约定可能在规划已经正确完成后仍被破坏。把扇出输出为可执行数据后，roster 宽度和 prompt 保真度由程序保证，同时保留现有 brief、覆盖率核算和 findings 流程。
对于 territory 扇出和 worktree review，本命令会 fail closed，因为 Step 3A workflow 目前无法安全表达这些执行契约。
Reviewer Test Plan
如何验证
在
packages/cli中运行npx vitest run src/commands/review.test.ts src/commands/review/emit-workflow.test.ts src/commands/review/workflow-script.test.ts，预期 22 条测试全部通过。然后在仓库根目录使用普通 local 或 diff-only Step 1 plan 运行npm run dev -- review emit-workflow --plan <step-1-plan.json> --out <empty-output-directory>。预期命令打印所需 agent 数量以及script.js、args.json的路径；参数文件包含每个必需 roster key 及其未经改写的启动 prompt，而标准输出不会打印 prompt 内容。使用 territory plan 或 PR-worktree plan 时，命令应说明拒绝原因并指向agent-prompt --roster，而不是生成可能误导的 workflow。证据（前后对比）
N/A——这是新增的内部 review helper，目前尚未接入用户可见的 review 行为。
测试平台
环境（可选）
Node.js 25.9.0、npm 11.12.1、无 sandbox。
风险与范围
关联 Issue
属于 #8769 的一部分。