feat(core): trust a generated-scripts root for workflow scriptPath loads - #9987
Conversation
`Workflow({scriptPath})` and `workflow({scriptPath})` only load files that
resolve inside the two saved-workflow directories, and every file in those
directories is also a `/<name>` slash command. A tool that generates a
workflow script for a single run therefore had no place to put it: writing
into `.qwen/workflows` hands the user a permanent command for a run that is
already over, and any other path is refused by the loader.
Add `<projectDir>/workflows/generated` as a third trusted root for
`{scriptPath}` loads only. It lives in the runtime dir beside the run
snapshots and journals, is never enumerated by `listSavedWorkflows`, and
cannot be reached by `workflow('<name>')`. The same realpath boundary check
and symlinked-root refusal apply. The tool description stops claiming that
`scriptPath` accepts a path anywhere.
Part of QwenLM#8769.
Claude-Session: https://claude.ai/code/session_017cUwuTey4APA8wAyAM6ScS
|
|
|
@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，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent proposal was: add a runtime-dir root (
No blockers, no AGENTS.md violations, nothing to cut. Testing evidence — the PR's own CIUnattended run: PR code is not built or executed here; the signal below is the PR's own CI on the reviewed commit, fetched via the API. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The unit suite is still running on the reviewed commit — nothing red so far, but green is not yet attested; the table above updates in place once CI settles. The behavioral surface here is the loader's accept/refuse semantics, and the seven new unit tests pin it by construction: each builds files under the new root (or its refusal cases), so without the change they would throw or enumerate differently — a suite that passes identically with the diff removed is not what this is. The PR's test plan reports 194 passed across the touched suites locally; on this unattended path CI is the signal I can attest to, and it supersedes that report. Nothing user-visible changes (no TUI surface), so no real-scenario capture applies here. 中文说明代码审查看 diff 之前我的独立方案是：在 runtime 目录加一个根（
无阻塞项、无 AGENTS.md 违规、无可删减。 测试证据 —— 本 PR 自己的 CI无人值守运行：此处不构建、不执行 PR 代码；上方信号来自 API 拉取的、被审提交上 PR 自己的 CI。单元测试仍在被审提交上运行，目前无失败，最终结论以表格就地更新为准。行为面（loader 的接受/拒绝语义）由 7 个新单元测试按构造钉死：每个测试都在新根下造文件（或构造其拒绝场景），缺了这个改动它们必然抛错或枚举不同。PR 测试计划自报本地 194 通过，无人值守路径上以 CI 为准。无用户可见改动（无 TUI 面），不适用真实场景捕获。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-pinned change; the only reservation is structural (a root with no writer yet), and CI has not settled. Stepping back: this is the third staged prerequisite in the #8769 series, and it reads like the two that already merged — small, precisely scoped, test-heavy. My independent proposal (widen only the The honest reservation: nothing writes to the new root yet, so its value arrives with the follow-up writer(s). That is a series-staging observation, not a code defect — the same pattern landed in #8971/#8972, and settling the loader's semantics (including the refusal message) before writers build on it is the safer order. If the #8769 direction stalls, this root is dead weight — but it is 68 production lines in one module, cheap to remove, and the parent proposal is active. Verdict: approve. The unit suites ( 中文说明信心：4/5 —— 干净、最小化、测试钉得牢；唯一的保留意见是结构性的（根目录尚无写入方），且 CI 尚未定局。 退一步看：这是 #8769 系列的第三个阶段性前置，风格与已合并的两个一致——小、边界精确、测试密集。我的独立方案（只放宽 诚实的保留意见：目前还没有任何写入方写这个新根，价值要等后续写入方落地。这是系列节奏的观察，不是代码缺陷——#8971/#8972 用了同样的模式，且先定 loader 语义（包括拒绝文案）再让写入方构建其上，是更稳的顺序。如果 #8769 方向停滞，这个根就成了死代码——但它只是一个模块里的 68 行生产代码，移除成本低，且父提案仍在推进。 结论：批准。单元测试（ — Qwen Code · qwen3.8-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short.; "agent 6c": none — all checks above ran to completion..
Test Plan (not a blocker): workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21452, 1702, 23916, 1659, 601, 4226, 627 passed; 784 passed — this review observed 21452, 1702, 23916, 1659, 601, 4226, 627 passed.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 4"：none — no check was cut short.；"agent 6c"：none — all checks above ran to completion.。
Test Plan（非阻断）：workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21452, 1702, 23916, 1659, 601, 4226, 627 passed; 784 passed — this review observed 21452, 1702, 23916, 1659, 601, 4226, 627 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| '`~/.qwen/workflows`) or the generated-scripts root ' + | ||
| '(`<projectDir>/workflows/generated`) — any other path is refused. ' + |
There was a problem hiding this comment.
[Suggestion] The model-facing description names the new trusted root as <projectDir>/workflows/generated, but <projectDir> is never defined in model-visible text, while the same paragraph uses <projectRoot> for the workspace (<projectRoot>/.qwen/workflows). The actual trusted root is Storage.getProjectDir() — the per-project runtime session-storage directory, outside the workspace tree. The same ambiguity appears in the Runtime paragraph further down. The concrete cost: a writer reading this description literally — the exact consumer this PR exists for — writes <workspace>/workflows/generated/fanout.js and hands it to Workflow({scriptPath}); the loader refuses it, the run fails, and the refusal is misleading because the refused path literally contains workflows/generated. A plausible workaround (dropping the script into .qwen/workflows/ instead) pollutes the slash-command namespace — the exact outcome this feature exists to prevent. Enforcement is code-side, so this ambiguity causes refusal, never a trust bypass. Suggested wording (apply the same to the Runtime paragraph):
'(`<projectDir>/workflows/generated`) — any other path is refused. ' +
→
'(`$QWEN_CODE_PROJECT_DIR/workflows/generated` — the per-project runtime dir, not the project tree) — any other path is refused. ' +
中文说明
模型可见的描述将新的受信根目录写作 <projectDir>/workflows/generated，但 <projectDir> 在模型可见文本中从未定义，而同一段落用 <projectRoot> 表示工作区（<projectRoot>/.qwen/workflows）。实际的受信根是 Storage.getProjectDir() —— 每个项目的 runtime 会话存储目录，位于项目树之外。下方的 Runtime 段落存在同样的歧义。具体代价：按字面理解该描述的写入方（正是本 PR 面向的消费者）会写出 <workspace>/workflows/generated/fanout.js 并交给 Workflow({scriptPath})，loader 将拒绝加载，运行失败，且该拒绝具有误导性——被拒路径本身包含 workflows/generated。一个可能的绕路做法（把脚本放进 .qwen/workflows/）会污染 slash command 命名空间——这正是本功能要避免的结果。边界检查在代码侧，因此该歧义只会导致拒绝，不会造成信任绕过。建议措辞如下（同样应用于 Runtime 段落）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| * out of the project tree. Layout below the root is the writer's; the | ||
| * loader trusts the whole subtree. A subprocess reaches it as | ||
| * `$QWEN_CODE_PROJECT_DIR/workflows/generated`. |
There was a problem hiding this comment.
[Suggestion] The subprocess reachability contract this docstring introduces — $QWEN_CODE_PROJECT_DIR + workflows/generated equals Storage.getGeneratedWorkflowsDir() — is asserted by no test; each side is tested only in isolation (shellContextEnv.test.ts asserts literal passthrough, and the new tests derive the path from the same Storage method the loader uses). If either side moves — e.g. the generated root is relocated under getProjectTempDir(), or the session-project-dir export changes — a tool that emits its one-run script to the documented path gets refused at load time, the workflow run fails, and the suite stays green because both halves still pass their own tests. Consider one effect-shaped assertion composing the two halves: register a session project dir, resolve getShellContextEnvVars(), write a script to path.join(env['QWEN_CODE_PROJECT_DIR']!, 'workflows', 'generated', 'x.js'), and assert resolveSavedWorkflowScript({ scriptPath: <that path> }, config) loads it.
中文说明
该 docstring 引入的子进程可达性契约——$QWEN_CODE_PROJECT_DIR + workflows/generated 等于 Storage.getGeneratedWorkflowsDir()——没有任何测试断言。两侧目前只被各自独立测试（shellContextEnv.test.ts 只断言字面量透传；新增测试从 loader 所用的同一个 Storage 方法推导路径）。若任一侧变动——例如 generated 根目录被迁到 getProjectTempDir() 下，或 session-project-dir 的导出方式改变——按文档路径写入一次性脚本的工具会在加载时被拒绝，workflow 运行失败，而测试套件仍为绿色（因为两侧各自的测试仍然通过）。建议增加一个端到端断言：注册 session 项目目录，解析 getShellContextEnvVars()，将脚本写入 path.join(env['QWEN_CODE_PROJECT_DIR']!, 'workflows', 'generated', 'x.js')，并断言 resolveSavedWorkflowScript({ scriptPath: <该路径> }, config) 能加载它。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!inside) { | ||
| throw new Error( | ||
| `refusing to load a workflow file outside the saved-workflow directories: '${filePath}'.`, | ||
| `refusing to load a workflow file outside the saved-workflow and generated-workflow directories: '${filePath}'.`, |
There was a problem hiding this comment.
[Suggestion] The refusal message names the directory kinds but not the roots actually checked, and this diff adds the first root whose location is not common knowledge — it resolves through the runtime-base fallback chain (pinned context > QWEN_RUNTIME_DIR > configured runtime base > global Qwen dir > os.tmpdir()/.qwen). A debugger facing this refusal sees only the offending path, checks the plausible-looking <projectRoot>/workflows/generated (absent — the root lives in the runtime dir), and must then read Storage source to locate the real root. dirs is already in scope at the throw site, so including it is free.
| `refusing to load a workflow file outside the saved-workflow and generated-workflow directories: '${filePath}'.`, | |
| `refusing to load a workflow file outside the workflow script roots (checked: ${dirs.join(', ')}): '${filePath}'.`, |
中文说明
拒绝信息只说明了目录类别，没有列出实际检查的根目录；而本 diff 新增了第一个位置不属于常识的根目录——它经由 runtime base 回退链解析（pinned context > QWEN_RUNTIME_DIR > 配置的 runtime base > 全局 Qwen 目录 > os.tmpdir()/.qwen）。调试者遇到该拒绝时只能看到被拒路径，会先检查貌似合理的 <projectRoot>/workflows/generated（并不存在——根目录在 runtime 目录中），随后不得不阅读 Storage 源码才能定位真正的根目录。dirs 在抛出点作用域内已存在，将其包含进来没有成本。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21485, 1702, 23930, 1659, 601, 4226, 627 passed; 784 passed — this review observed 21485, 1702, 23930, 1659, 601, 4226, 627 passed.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/config/storage.ts:695 — [review] consumer half (trust root + model-facing contract) landed with no producer/writer in the treepackages/core/src/tools/workflow/workflow.ts:184 — [probe] generated-root scriptPath gets a strictly weaker consent gate than an inline script (path-only dialog, pre-approval offered)packages/core/src/agents/runtime/workflow-saved.ts:102 — [review] nested workflow({scriptPath}) route reaches the generated root with no consent dialogpackages/core/src/config/storage.ts:695 — [probe] generated root keyed on the lossy sanitizeCwd id — path-colliding projects share one trust root
中文说明
已审查。 建议见行内评论。
Test Plan（非阻断）：workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21485, 1702, 23930, 1659, 601, 4226, 627 passed; 784 passed — this review observed 21485, 1702, 23930, 1659, 601, 4226, 627 passed。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| 'this run hands you its path the same way. The file must resolve ' + | ||
| 'inside a saved-workflow directory (`.qwen/workflows`, ' + | ||
| '`~/.qwen/workflows`) or the generated-scripts root ' + | ||
| '(`<projectDir>/workflows/generated`) — any other path is refused. ' + |
There was a problem hiding this comment.
[Suggestion] R1-1: The model-facing description names the new trusted root as <projectDir>/workflows/generated, but <projectDir> is never defined in model-visible text, while the same paragraph uses <projectRoot> for the workspace (<projectRoot>/.qwen/workflows). The actual trusted root is Storage.getProjectDir() — the per-project runtime session-storage directory, outside the workspace tree. The same ambiguity appears in the Runtime paragraph further down.
Re-checked at the round-2 head da095844 — the description text is unchanged since round 1, so this still stands. The concrete cost: a writer reading this description literally — the exact consumer this PR exists for — writes <workspace>/workflows/generated/fanout.js and hands it to Workflow({scriptPath}); the loader refuses it, the run fails, and the refusal is misleading because the refused path literally contains workflows/generated. A plausible workaround (dropping the script into .qwen/workflows/ instead) pollutes the slash-command namespace — the exact outcome this feature exists to prevent. Enforcement is code-side, so this ambiguity causes refusal, never a trust bypass. Verified by probe this round (scratch tree at the reviewed commit):
projectRoot : /tmp/probe-proj-c89w82
generatedDir: /tmp/probe-home-lI3laY/.qwen/projects/-tmp-probe-proj-c89w82/workflows/generated
generatedDir inside projectRoot? false
Suggested wording (apply the same to the Runtime paragraph):
'(`<projectDir>/workflows/generated`) — any other path is refused. ' +
→
'(`$QWEN_CODE_PROJECT_DIR/workflows/generated` — the per-project runtime dir, not the project tree) — any other path is refused. ' +
中文说明
模型可见的描述将新的受信根目录写作 <projectDir>/workflows/generated，但 <projectDir> 在模型可见文本中从未定义，而同一段落用 <projectRoot> 表示工作区（<projectRoot>/.qwen/workflows）。实际的受信根是 Storage.getProjectDir() —— 每个项目的 runtime 会话存储目录，位于项目树之外。下方的 Runtime 段落存在同样的歧义。
已在 round 2 头提交 da095844 复查——描述文本自 round 1 以来未变，该问题仍然存在。具体代价：按字面理解该描述的写入方（正是本 PR 面向的消费者）会写出 <workspace>/workflows/generated/fanout.js 并交给 Workflow({scriptPath})，loader 将拒绝加载，运行失败，且该拒绝具有误导性——被拒路径本身包含 workflows/generated。一个可能的绕路做法（把脚本放进 .qwen/workflows/）会污染 slash command 命名空间——这正是本功能要避免的结果。边界检查在代码侧，因此该歧义只会导致拒绝，不会造成信任绕过。本轮已通过探针验证（在受审提交的独立工作树中运行）。
建议措辞如下（同样应用于 Runtime 段落）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| * business in the user's command namespace, and the runtime dir keeps it | ||
| * out of the project tree. Layout below the root is the writer's; the | ||
| * loader trusts the whole subtree. A subprocess reaches it as | ||
| * `$QWEN_CODE_PROJECT_DIR/workflows/generated`. |
There was a problem hiding this comment.
[Suggestion] R1-2: The subprocess reachability contract this docstring introduces — $QWEN_CODE_PROJECT_DIR + workflows/generated equals Storage.getGeneratedWorkflowsDir() — is asserted by no test; each side is tested only in isolation (shellContextEnv.test.ts asserts literal passthrough, and the new tests derive the path from the same Storage method the loader uses).
Re-checked at the round-2 head da095844 — the code is unchanged since round 1, and a fresh sweep confirms the gap still exists: the literal workflows/generated appears in zero test files, and no test ties QWEN_CODE_PROJECT_DIR to the generated dir. If either side moves — e.g. the generated root is relocated under getProjectTempDir(), or the session-project-dir export changes — a tool that emits its one-run script to the documented path gets refused at load time, the workflow run fails, and the suite stays green because both halves still pass their own tests. Consider one effect-shaped assertion composing the two halves: register a session project dir, resolve getShellContextEnvVars(), write a script to path.join(env['QWEN_CODE_PROJECT_DIR']!, 'workflows', 'generated', 'x.js'), and assert resolveSavedWorkflowScript({ scriptPath: <that path> }, config) loads it.
中文说明
该 docstring 引入的子进程可达性契约——$QWEN_CODE_PROJECT_DIR + workflows/generated 等于 Storage.getGeneratedWorkflowsDir()——没有任何测试断言。两侧目前只被各自独立测试（shellContextEnv.test.ts 只断言字面量透传；新增测试从 loader 所用的同一个 Storage 方法推导路径）。
已在 round 2 头提交 da095844 复查——代码自 round 1 以来未变，新一轮扫描确认缺口仍在：workflows/generated 字面量在测试文件中零出现，也没有测试将 QWEN_CODE_PROJECT_DIR 与 generated 目录关联。若任一侧变动——例如 generated 根目录被迁到 getProjectTempDir() 下，或 session-project-dir 的导出方式改变——按文档路径写入一次性脚本的工具会在加载时被拒绝，workflow 运行失败，而测试套件仍为绿色（因为两侧各自的测试仍然通过）。建议增加一个端到端断言：注册 session 项目目录，解析 getShellContextEnvVars()，将脚本写入 path.join(env['QWEN_CODE_PROJECT_DIR']!, 'workflows', 'generated', 'x.js')，并断言 resolveSavedWorkflowScript({ scriptPath: <该路径> }, config) 能加载它。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!inside) { | ||
| throw new Error( | ||
| `refusing to load a workflow file outside the saved-workflow directories: '${filePath}'.`, | ||
| `refusing to load a workflow file outside the saved-workflow and generated-workflow directories: '${filePath}'.`, |
There was a problem hiding this comment.
[Suggestion] R1-3: The refusal message names the directory kinds but not the roots actually checked, and this diff adds the first root whose location is not common knowledge — it resolves through the runtime-base fallback chain (pinned context > QWEN_RUNTIME_DIR > configured runtime base > global Qwen dir > os.tmpdir()/.qwen). A debugger facing this refusal sees only the offending path, checks the plausible-looking <projectRoot>/workflows/generated (absent — the root lives in the runtime dir), and must then read Storage source to locate the real root. dirs is already in scope at the throw site, so including it is free.
Re-checked at the round-2 head da095844 — the message is unchanged since round 1, so this still stands; a probe this round reproduced the refusal and confirmed the message names none of the three checked roots (fix-flip verified: appending the roots flips the check to true).
| `refusing to load a workflow file outside the saved-workflow and generated-workflow directories: '${filePath}'.`, | |
| `refusing to load a workflow file outside the workflow script roots (checked: ${dirs.join(', ')}): '${filePath}'.`, |
中文说明
拒绝信息只说明了目录类别，没有列出实际检查的根目录；而本 diff 新增了第一个位置不属于常识的根目录——它经由 runtime base 回退链解析（pinned context > QWEN_RUNTIME_DIR > 配置的 runtime base > 全局 Qwen 目录 > os.tmpdir()/.qwen）。调试者遇到该拒绝时只能看到被拒路径，会先检查貌似合理的 <projectRoot>/workflows/generated（并不存在——根目录在 runtime 目录中），随后不得不阅读 Storage 源码才能定位真正的根目录。dirs 在抛出点作用域内已存在，将其包含进来没有成本。
已在 round 2 头提交 da095844 复查——拒绝信息自 round 1 以来未变，该问题仍然存在；本轮探针复现了该拒绝，并确认信息未列出三个被检查根目录中的任何一个（修复翻转已验证：将根目录追加进信息后检查变为 true）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…roots (QwenLM#9987) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summaryAll three open suggestions (R1-1, R1-2, R1-3 — each raised in review rounds 1 and 2) are addressed in one commit. No Critical or Request-changes findings were open. No conflict resolution was needed ( Feedback pointsR1-1 — Model-facing description names the trusted root as
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21486, 1702, 23927, 1659, 601, 4226, 627 passed; 784 passed — this review observed 21486, 1702, 23927, 1659, 601, 4226, 627 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/agents/runtime/workflow-saved.ts:102 — [probe] generated-root scripts are model/tool-authored but inherit scriptPath's path-only consent and grantable always-allow rule — consent granted for one file's content silently tra…
Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 3 (0 new). Findings keep coming back to the same files: packages/core/src/agents/runtime/workflow-saved.ts (findings in round 1; 1 more now); packages/core/src/tools/workflow/workflow.ts (findings in round 1; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
Test Plan（非阻断）：workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21486, 1702, 23927, 1659, 601, 4226, 627 passed; 784 passed — this review observed 21486, 1702, 23927, 1659, 601, 4226, 627 passed。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 3 条（其中 0 条首次提出）。发现反复回到同一批文件：packages/core/src/agents/runtime/workflow-saved.ts（第 1 轮已出过发现，本轮又有 1 条）；packages/core/src/tools/workflow/workflow.ts（第 1 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| **Runtime** — see the \`script\` parameter for the detailed authoring contract. | ||
|
|
||
| \`phase(title)\`, \`log(msg)\`, \`agent(prompt, opts?)\`, \`parallel(thunks)\`, \`pipeline(items, ...stages)\`, \`workflow(nameOrRef, args?)\`, plus the \`args\` and \`budget\` globals. \`workflow()\` runs a saved workflow inline under this run's caps and nests one level only — a workflow reached through \`workflow()\` cannot call \`workflow()\` itself, and doing so throws. Saved workflows are \`<name>.js\` files under \`<projectRoot>/.qwen/workflows\` (project scope, also surfaced as \`/<name>\` slash commands) or \`~/.qwen/workflows\` (user scope, lower precedence when both define the same name); \`workflow('<name>')\` resolves against those two directories, while \`scriptPath\` takes an absolute path to a script anywhere. Default \`max(1, min(16, cpus-2))\` agents in flight per run (\`${MAX_WORKFLOW_CONCURRENCY_ENV}\`), up to ${DEFAULT_MAX_AGENTS_PER_RUN} agents total (\`${MAX_WORKFLOW_AGENTS_ENV}\`), under a 30-minute wall-clock cap per run (\`QWEN_CODE_MAX_WORKFLOW_SECONDS\`) — a fan-out near the agent cap will not fit inside the default cap. Each subagent attempt is separately capped at ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS} turns (\`${WORKFLOW_SUBAGENT_MAX_TURNS_ENV}\`) and ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES} minutes (\`${WORKFLOW_SUBAGENT_MAX_MINUTES_ENV}\`) — an attempt that hits either becomes \`null\` in \`parallel()\`/\`pipeline()\`, indistinguishable from a missing agent, so raise them for legitimately long work. A per-run output-token cap may also be in effect: read \`budget.total\` (\`null\` = uncapped) before committing to a large fan-out, because once the cap is reached every further \`agent()\` call is refused — a bare sequential \`await agent()\` sees the rejection, while inside \`parallel()\`/\`pipeline()\` the refused slot becomes \`null\` and the script keeps running on partial results. Per-call \`agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs })\` covers structured-output contracts, declarative-agent selection, model override, git-worktree-isolated subagents, pinning an agent to a caller-owned worktree, and the no-progress stall watchdog (\`stallMs: 0\` disables it). \`resumeFromRunId\` resumes a prior run — agent() calls whose rolling prefix-hash matches the journal are served from cache for the longest unchanged prefix. Runs appear in the background-tasks view and the \`/workflows\` dialog (live phase tree, token usage, cooperative pause/resume, cancel); \`run_in_background: true\` returns a run handle immediately in the interactive TUI and delivers completion through the conversation. Scripts run in a node:vm sandbox with no filesystem or shell access — all I/O happens through the spawned agents. | ||
| \`phase(title)\`, \`log(msg)\`, \`agent(prompt, opts?)\`, \`parallel(thunks)\`, \`pipeline(items, ...stages)\`, \`workflow(nameOrRef, args?)\`, plus the \`args\` and \`budget\` globals. \`workflow()\` runs a saved workflow inline under this run's caps and nests one level only — a workflow reached through \`workflow()\` cannot call \`workflow()\` itself, and doing so throws. Saved workflows are \`<name>.js\` files under \`<projectRoot>/.qwen/workflows\` (project scope, also surfaced as \`/<name>\` slash commands) or \`~/.qwen/workflows\` (user scope, lower precedence when both define the same name); \`workflow('<name>')\` resolves against those two directories, while \`scriptPath\` takes an absolute path to a script inside either of them or inside the generated-scripts root (\`$QWEN_CODE_PROJECT_DIR/workflows/generated\` — the per-project runtime dir, not the project tree — where a tool emitting a one-run script writes it; never a slash command, never resolvable by name); a path outside those roots is refused. Default \`max(1, min(16, cpus-2))\` agents in flight per run (\`${MAX_WORKFLOW_CONCURRENCY_ENV}\`), up to ${DEFAULT_MAX_AGENTS_PER_RUN} agents total (\`${MAX_WORKFLOW_AGENTS_ENV}\`), under a 30-minute wall-clock cap per run (\`QWEN_CODE_MAX_WORKFLOW_SECONDS\`) — a fan-out near the agent cap will not fit inside the default cap. Each subagent attempt is separately capped at ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS} turns (\`${WORKFLOW_SUBAGENT_MAX_TURNS_ENV}\`) and ${DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES} minutes (\`${WORKFLOW_SUBAGENT_MAX_MINUTES_ENV}\`) — an attempt that hits either becomes \`null\` in \`parallel()\`/\`pipeline()\`, indistinguishable from a missing agent, so raise them for legitimately long work. A per-run output-token cap may also be in effect: read \`budget.total\` (\`null\` = uncapped) before committing to a large fan-out, because once the cap is reached every further \`agent()\` call is refused — a bare sequential \`await agent()\` sees the rejection, while inside \`parallel()\`/\`pipeline()\` the refused slot becomes \`null\` and the script keeps running on partial results. Per-call \`agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs })\` covers structured-output contracts, declarative-agent selection, model override, git-worktree-isolated subagents, pinning an agent to a caller-owned worktree, and the no-progress stall watchdog (\`stallMs: 0\` disables it). \`resumeFromRunId\` resumes a prior run — agent() calls whose rolling prefix-hash matches the journal are served from cache for the longest unchanged prefix. Runs appear in the background-tasks view and the \`/workflows\` dialog (live phase tree, token usage, cooperative pause/resume, cancel); \`run_in_background: true\` returns a run handle immediately in the interactive TUI and delivers completion through the conversation. Scripts run in a node:vm sandbox with no filesystem or shell access — all I/O happens through the spawned agents. |
There was a problem hiding this comment.
[Suggestion] The approval/activity surface still labels every scriptPath run as a saved workflow: resolveMeta() returns null whenever script is absent, so buildConfirmationPrompt prints Saved workflow: <path> and getDescription() prints Run saved workflow (<basename>) — for generated-root scripts too. That presents a throwaway generated artifact as a persistent user-saved command, the opposite of the distinction this same description draws ("never a slash command, never resolvable by name"): a user who sees "Saved workflow: …/workflows/generated/x.js" approves (or pre-approves the path rule) under a misidentification of what the artifact is. Verified by probe at this commit: a generated-root scriptPath renders Run saved workflow (fanout-1a2b3c.js) / Saved workflow: …/workflows/generated/fanout-1a2b3c.js, byte-identical to a real saved workflow's labels. Consider a neutral label for scriptPath runs without saved-workflow meta — e.g. Workflow script: <path> / Run workflow script (<basename>) in getDescription() and buildConfirmationPrompt, or detect the generated root via Storage.getGeneratedWorkflowsDir() and print Generated workflow script: <path>.
中文说明
审批/活动界面仍然把每一次 scriptPath 运行标注为"已保存的 workflow"：只要 script 缺省，resolveMeta() 就返回 null，于是 buildConfirmationPrompt 打印 Saved workflow: <path>、getDescription() 打印 Run saved workflow (<basename>) —— 对 generated 根目录下的脚本也是如此。这会把一次性的生成产物呈现为用户持久保存的命令，与这段描述自己所划的界限（"never a slash command, never resolvable by name"）正好相反：用户看到 "Saved workflow: …/workflows/generated/x.js" 时，是在对产物身份的误认下批准（或预批准该路径规则）。已在本次提交上用探针验证：generated 根的 scriptPath 渲染出 Run saved workflow (fanout-1a2b3c.js) / Saved workflow: …/workflows/generated/fanout-1a2b3c.js，与真正的已保存 workflow 标签逐字节相同。建议对没有 saved-workflow 元信息的 scriptPath 运行改用中性标签——例如在 getDescription() 与 buildConfirmationPrompt 中使用 Workflow script: <path> / Run workflow script (<basename>)，或通过 Storage.getGeneratedWorkflowsDir() 识别 generated 根并打印 Generated workflow script: <path>。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!inside) { | ||
| throw new Error( | ||
| `refusing to load a workflow file outside the saved-workflow directories: '${filePath}'.`, | ||
| `refusing to load a workflow file outside the workflow script roots (checked: ${dirs.join(', ')}): '${filePath}'.`, |
There was a problem hiding this comment.
[Suggestion] The new "(checked: …)" list can omit one of the three roots: a root that is itself a symlink maps to null inside readWorkflowFileSecurely's Promise.all and is dropped by the filter before dirs — and the message — are built. Verified by probe at this commit: with the generated root symlinked (<runtime>/workflows/generated -> /elsewhere, the exact case the new "refuses a symlinked generated root" test exercises) the refusal reads outside the workflow script roots (checked: <project>/.qwen/workflows, <user>/.qwen/workflows) — the generated root absent — while the non-symlinked control lists all three. A debugger hitting this refusal concludes the loader never considered the generated root and "fixes" things by moving the file into .qwen/workflows, when the actionable cause is the symlinked root itself, which the message never hints at — defeating the stated point of listing the checked roots. Keep refused-but-considered roots visible, e.g. collect the symlinked ones separately instead of discarding them in the filter, and mark them in the message: (checked: <project>/.qwen/workflows, <user>/.qwen/workflows; refused symlinked root: <runtime>/workflows/generated).
中文说明
新的 "(checked: …)" 列表可能漏掉三个根目录之一：如果某个根本身是软链接，它会在 readWorkflowFileSecurely 的 Promise.all 中被映射为 null，并在 dirs（以及这条错误信息）构建之前被 filter 丢弃。已在本次提交上用探针验证：当 generated 根是软链接时（<runtime>/workflows/generated -> /elsewhere，正是新增测试 "refuses a symlinked generated root" 覆盖的场景），拒绝信息为 outside the workflow script roots (checked: <project>/.qwen/workflows, <user>/.qwen/workflows) —— generated 根不在其中——而非软链接的对照组会列出全部三个根。调试者看到这条拒绝信息会认为 loader 从未考虑过 generated 根，于是把文件挪进 .qwen/workflows 来"修复"，而真正可处置的原因是软链接根目录本身——信息对此毫无提示，这恰恰落空了"列出被检查的根"这一修改的初衷。建议保留"被考虑但被拒绝"的根目录：不要把软链接根在 filter 中丢弃，而是单独收集，并在信息中标注，例如：(checked: <project>/.qwen/workflows, <user>/.qwen/workflows; refused symlinked root: <runtime>/workflows/generated)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Round 4 review
Reviewed at 98863f0 (current head, a merge of upstream/main into the feature branch). Key changes since round 3 (at 3a555a182f): only a merge commit — no new code changes since the round-3 review.
Previous findings status
R1-1 (Suggestion) — workflow.ts description named <projectDir> (undefined in model-visible text). Fixed in autofix round 1 at 3a555a182f: now names $QWEN_CODE_PROJECT_DIR/workflows/generated with a clarifying note.
R1-2 (Suggestion) — No test asserted the $QWEN_CODE_PROJECT_DIR subprocess reachability contract. Fixed in autofix round 1: new composed contract test witnesses both halves of the contract.
R1-3 (Suggestion) — Refusal message named directory kinds but not the actual roots checked. Fixed in autofix round 1: now lists the checked roots inline.
R3-1 (Suggestion, still open) — workflow.ts:852 — Approval/activity surface still labels every scriptPath run as a "saved workflow" even when loaded from the generated root. The consumption side (activity entry, consent dialog) hasn't been updated to distinguish the generated root. Deferred under convergence posture — the PR only adds the loader; the consumer-writer pairing arrives in a follow-up.
R3-2 (Suggestion, still open) — workflow-saved.ts:190 — The (checked: …) list can omit a root when that root is a symlink (the isSymlinkedDir filter drops it silently). The error message shows the remaining roots without explaining the omission. Minor: the user sees (checked: /a, /b) and may wonder why /c isn't checked. Adding a note like (symlinked-root /c: excluded from check) would clarify.
New findings (round 4)
No new Criticals, no new Suggestions. The code is clean, the merge commit introduces no conflicts, and the behavior is well-pinned by tests.
Limitations of this round
- No worktree could be created (
git fetchblocked bygetaddrinfoon this Windows machine). Review is based on the diff fromgh pr diff. - No build, typecheck, or test run was performed.
- Round 4 (this review) is a diff-only pass.
Convergence: 0 Criticals open, 2 Suggestions open (both from round 3, neither a blocker). Merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending.
— via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Round 5 review (second opinion)
Reviewed at 98863f0 — head unchanged since round 4. Independent re-review pass.
CI state changed since round 4 — base-induced, not this PR
Both Test jobs are red on this head, but neither failure is in this PR's code path:
- ubuntu:
permission-manager.test.ts(resolveToolNameexhaustiveness over thereport_findingsalias) — the branch last merged main at 12:51, and main itself carried that inconsistency untila6d30ebc("fix(core): register report_findings in the permission alias table") landed at 15:50, after this PR's CI ran. The next update-branch merge clears it. - windows:
daemon-git-worktree-guard*anddws-event-streamfailures — windows-lane breakage unrelated to this change.
This PR's own tests are green on both platforms: workflow-saved.test.ts 40/40 and storage.test.ts 101/101 on ubuntu, same on windows (including the new symlink-escape tests).
Previous findings status
- R1-1, R1-2, R1-3 — fixed in autofix round 1 (
3a555a182f); re-verified at head. - R3-1 (Suggestion, still open) — approval/activity surfaces still label generated-root loads as "saved workflow". For whoever takes it: the family is
getDescription()("Run saved workflow (…)") and the confirmation body ("Saved workflow: …"), plus theP7binterface comment and the XOR validation message inworkflow.ts. - R3-2 (Suggestion, still open) — a symlinked root is silently dropped from the
(checked: …)refusal list.
Independent checks added this round
- The run-artifact consumers of the workflow runs dir (
workflow-snapshot.ts) cannot collide with the newgenerated/subdirectory: snapshot listing filters.jsonfiles, and every recursive delete is gated on the^wf_[0-9a-f]+$run-id shape. - The subprocess reachability contract holds in production, not just in the new test:
Config.initializeregistersstorage.getProjectDir()andgetShellContextEnvVarsreads it back. - Slash-command discovery and name resolution never touch the generated root, and
scriptPathapproval grants are scoped to the exact path, so a grant for one generated script does not extend to any other file under the root. - No other code, tests, or docs reference the old refusal message.
Verdict
No Criticals, no new Suggestions. 0 Criticals / 2 Suggestions open (both from round 3, neither a blocker).
Diff-only pass again this round: git fetch remains blocked on this Windows machine, so no local worktree/build/test — the CI logs above stand in as test evidence.
— via Qwen Code /review
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9987Commit: Feedback points and dispositionsrc:3852905908 — R3-1 (Suggestion,
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
自动代码审查汇总（PR head 已漂移）
本次审查基于的 commit 为
aef0af85d3eb17f7115c0bf6065214dfcd7e4e52，当前 PR head 已更新为9254a2c58b938b98428c1d80f98f85ce8346dd19。由于 head 漂移，原 inline 评论可能无法准确锚定到行，故仅保留汇总。
结论
- 模型：deepseek-v4-flash
- 结果：0 Critical / 1 Suggestion
- 总体：COMMENT（建议修复后合并）
Findings
R3-2 — Suggestion：symlinked root 在错误消息中静默省略
当某个 workflow script root 本身是 symlink 时，代码会将其过滤掉（return null），导致它在错误消息的 (checked: …) 列表中被静默省略。用户看到 (checked: dir1, dir2) 时，无从得知第三个 root 因是 symlink 而被排除。
- 影响：调试体验 / 错误信息完整性
- 安全层面：该拒绝行为本身是正确的（symlinked root 被正确拒绝）
- 建议：在错误消息中显式说明被排除的 symlinked root，或至少保留占位提示，避免调试困惑
备注
- 本地 Windows runner 在创建 worktree 时遇到已知网络/DNS 问题（
getaddrinfo() thread failed to start），因此 qwen 通过gh api拉取 diff 完成审查，未自动生成 inline comments。 - 其余改动（新增
getWorkflowScriptRoots()、扩展 containment boundary、workflow.ts描述修正、新增 7 个单元测试）均符合预期，未发现新问题。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): workflows/generated-evil/x.js — no such file or directory.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/workflow/workflow.ts:326 — [probe] generated-root scripts inherit scriptPath's path-only consent and grantable always-allow rule — consent granted for one file's content silently transfers to whatever occupies the pa…
Convergence: round 4 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/core/src/tools/workflow/workflow.ts (findings in round 3; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
Test Plan（非阻断）：workflows/generated-evil/x.js — no such file or directory。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 4 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/core/src/tools/workflow/workflow.ts（第 3 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| function isGeneratedWorkflowScriptPath( | ||
| config: Config, | ||
| scriptPath: string, | ||
| ): boolean { | ||
| const root = config.storage.getGeneratedWorkflowsDir(); | ||
| return scriptPath === root || scriptPath.startsWith(root + path.sep); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R4-1: This classifier decides "generated" vs "saved" by a lexical prefix match on the raw scriptPath, while the loader (readWorkflowFileSecurely) decides loadability canonically via fs.realpath — so the label shown at approval can be the opposite of what actually loads. A scriptPath with .. segments (e.g. <generated-root>/../../../../workflows/audit.js) matches the generated prefix here, and the dialog says "Generated workflow script", but realpath normalizes it to ~/.qwen/workflows/audit.js and the loader runs the user's saved workflow — the approval is granted under the wrong identity, and the persisted rule Workflow(scriptPath:<raw ..-path>) never matches a later canonical invocation. The mirror corner also flips the label: on a host whose runtime base sits under a symlinked ancestor (macOS /tmp -> /private/tmp), a writer that canonicalizes the path labels a tool-generated script "Saved workflow". The security boundary itself holds — this is identity/disclosure, which this function exists to get right. Note the predicate also duplicates the shared isWithinRoot helper (fileUtils.ts), which already normalizes both sides.
Witness (probe on the unmodified PR code): raw .. scriptPath → description Run generated workflow script (audit.js) while the loader canonically loaded the saved script; persisted rule keeps the raw .. spelling.
Minimal fix — normalize both sides (this closes the .. class; the symlinked-ancestor corner needs deriving the label from the loader's canonical comparison):
| function isGeneratedWorkflowScriptPath( | |
| config: Config, | |
| scriptPath: string, | |
| ): boolean { | |
| const root = config.storage.getGeneratedWorkflowsDir(); | |
| return scriptPath === root || scriptPath.startsWith(root + path.sep); | |
| } | |
| function isGeneratedWorkflowScriptPath( | |
| config: Config, | |
| scriptPath: string, | |
| ): boolean { | |
| const root = path.resolve(config.storage.getGeneratedWorkflowsDir()); | |
| const candidate = path.resolve(scriptPath); | |
| return candidate === root || candidate.startsWith(root + path.sep); | |
| } |
中文说明
[Suggestion] R4-1：该分类器通过原始 scriptPath 的词法前缀匹配来判定 "generated" 与 "saved"，而 loader（readWorkflowFileSecurely）通过 fs.realpath 以规范化方式判定可加载性——因此审批界面显示的标签可能与实际加载的内容相反。带 .. 段的 scriptPath（例如 <generated-root>/../../../../workflows/audit.js）在此处会匹配 generated 前缀，对话框显示 "Generated workflow script"，但 realpath 会将其规范化为 ~/.qwen/workflows/audit.js，loader 实际运行的是用户保存的 workflow——审批在错误身份下被授予，且持久化规则 Workflow(scriptPath:<原始 ..-路径>) 永远无法匹配后续规范化的调用。镜像情形同样会翻转标签：在 runtime 根目录位于软链祖先之下的主机上（macOS /tmp -> /private/tmp），写入方若先规范化路径，会把工具生成的脚本标记为 "Saved workflow"。安全边界本身不受影响——这是身份/披露问题，而获取正确身份正是本函数存在的意义。另外该谓词与共享的 isWithinRoot helper（fileUtils.ts）重复，后者已对两侧做规范化。
证据（在未改动的 PR 代码上运行探针）：原始 .. scriptPath → 描述为 Run generated workflow script (audit.js)，而 loader 实际规范化加载了保存的脚本；持久化规则保留了原始 .. 拼写。
最小修复——对两侧做规范化（可消除 .. 一类问题；软链祖先情形需要从 loader 的规范化比较结果推导标签）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // A generated-root script is a throwaway artifact a tool emitted for this | ||
| // run. Labeling it as a saved workflow would have the user approve — and | ||
| // maybe pre-approve the path rule — under a wrong identity. | ||
| it('labels a generated-root scriptPath as a generated script, not a saved workflow', async () => { |
There was a problem hiding this comment.
[Suggestion] R4-2: The new label tests only pin scripts sitting directly under the generated root, but nested paths are the writer pattern this PR documents ("trusts the whole subtree", load-tested in workflow-saved.test.ts) — and a plausible one-line mutation survives: change return scriptPath === root || scriptPath.startsWith(root + path.sep); to return scriptPath === root || path.dirname(scriptPath) === root; and both label tests still pass, yet <generated-root>/s-abc/fanout.js is then labeled "Saved workflow" in the approval dialog — the wrong-identity approval the label exists to prevent. Witness: in a scratch-tree mutation run, both label tests passed with the mutant applied while a nested probe flipped from Run generated workflow script (fanout.js) to Run saved workflow (fanout.js); reverting the mutant flipped it back.
Add one nested-path assertion beside these tests, e.g.:
const scriptPath = path.join(
storage.getGeneratedWorkflowsDir(),
's-abc',
'fanout.js',
);
// expect getDescription() === 'Run generated workflow script (fanout.js)'
// and details.prompt to contain `Generated workflow script: ${scriptPath}`中文说明
[Suggestion] R4-2：新增的标签测试只钉住了直接位于 generated 根目录下的脚本，但嵌套路径才是本 PR 文档中描述的写入方布局（"信任整棵子树"，workflow-saved.test.ts 中已有加载测试）——而且一个合理的一行变异可以存活：把 return scriptPath === root || scriptPath.startsWith(root + path.sep); 改为 return scriptPath === root || path.dirname(scriptPath) === root;，两个标签测试仍然通过，但 <generated-root>/s-abc/fanout.js 会在审批对话框中被标记为 "Saved workflow"——正是该标签要防止的错误身份审批。证据：在临时树变异运行中，施加变异后两个标签测试仍通过，而嵌套探针从 Run generated workflow script (fanout.js) 翻转为 Run saved workflow (fanout.js)；还原变异后翻转回来。
在这些测试旁补一个嵌套路径断言即可（示例代码见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9987Commit: Feedback points and dispositionsrc:3856248687 — R4-1 (Suggestion,
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): workflows/generated-evil/x.js — no such file or directory.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/workflow/workflow.ts:186 — [review] generated-root contract prose (schema description + Runtime paragraph) is anchored by no test despite the file's prose-anchoring convention
Convergence: round 5 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/core/src/tools/workflow/workflow.test.ts (findings in round 4; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
Test Plan（非阻断）：workflows/generated-evil/x.js — no such file or directory。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 3 条行内评论，其中 2 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/core/src/tools/workflow/workflow.test.ts（第 4 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| config: Config, | ||
| scriptPath: string, | ||
| ): boolean { | ||
| return isWithinRoot(scriptPath, config.storage.getGeneratedWorkflowsDir()); |
There was a problem hiding this comment.
[Suggestion] R4-1: The round-4 fix landed as asked — the classifier now normalizes both sides through isWithinRoot, so the reported ..-laced misclassification is closed and the new test pins its witness shape. One branch of the same divergence remains: the classifier resolves paths lexically (path.resolve) while the loader decides loadability with fs.realpath, so a scriptPath whose spelling diverges from its realpath gets the opposite provenance label from the content that actually loads — and the doc comment rewritten in this commit still says a path is classified at "the same location the loader decides to load", which holds for .. segments but not for symlinks.
Concrete trigger, reproduced by a probe against this commit: <projectDir>/workflows/generated/run.js shipped as a symlink to <projectRoot>/.qwen/workflows/deploy.js — both paths sit inside trusted roots, so the loader accepts — the approval dialog prints Generated workflow script: …run.js (a throwaway-artifact identity) while the content that loads is the durable saved workflow also surfaced as /deploy; the mirror spelling labels a loaded generated script Saved workflow; and a symlinked ancestor of the runtime dir (the macOS /tmp → /private/tmp class) makes a loaded generated script read as Saved workflow too.
Impact is confined to the label: the permission rule keys on the raw scriptPath, hideAlwaysAllow on inline-script-ness, and the loader's realpath boundary still makes the security decision, so no grant is widened — but the label is exactly what this change exists to get right. Either classify best-effort with the same canonicalization the loader uses (fs.realpath of both sides, with a lexical fallback for a not-yet-existing path; the confirmation surface is already async), or — if the lexical design is deliberate — correct the doc comment to say the classifier normalizes .. lexically only and may disagree with the loader across symlinks.
Witness — probe on unmodified PR code driving the real loader and label surfaces with real symlinks:
CASE1 symlink <genRoot>/run.js -> <savedRoot>/deploy.js
loader: ACCEPTED, loaded content = the saved workflow
dialog: Generated workflow script: <genRoot>/run.js
CASE2 symlink <savedRoot>/toolgen.js -> <genRoot>/s-abc/throwaway.js
loader: ACCEPTED, loaded content = the generated script
dialog: Saved workflow: <savedRoot>/toolgen.js
Applying the suggested realpath classification flipped both labels to match the loaded content, with all 58 PR tests still passing.
中文说明
[Suggestion] R4-1：第 4 轮的修复已按要求落地——分类器现在通过 isWithinRoot 对两侧做归一化，因此当初报告的含 .. 路径误分类已被封闭，新增测试也钉住了该见证场景。同一分歧还剩一个分支：分类器以词法方式解析路径（path.resolve），而 loader 用 fs.realpath 决定可加载性，因此拼写与其 realpath 不一致的 scriptPath 会得到与实际加载内容相反的来源标签——而本次提交重写的文档注释仍声称路径会在「loader 决定加载的同一位置」被分类；这对 .. 片段成立，对符号链接不成立。
具体触发场景（已通过针对本提交的探针复现）：<projectDir>/workflows/generated/run.js 以符号链接形式指向 <projectRoot>/.qwen/workflows/deploy.js——两个路径都在受信根内，因此 loader 接受加载——审批对话框显示 Generated workflow script: …run.js（一次性产物的身份），而实际加载的内容是同时以 /deploy 呈现的持久化已保存 workflow；反向拼写则会把加载的生成脚本标成 Saved workflow；runtime 目录的符号链接祖先目录（macOS /tmp → /private/tmp 一类）同样会让已加载的生成脚本显示为 Saved workflow。
影响仅限于标签：权限规则以原始 scriptPath 为键，hideAlwaysAllow 取决于是否内联脚本，安全决策仍由 loader 的 realpath 边界做出，因此没有任何授权被放宽——但标签正是本次改动要修正的东西。要么按与 loader 相同的规范化方式做尽力而为的分类（对两侧取 fs.realpath，对尚不存在的路径回退到词法解析；确认路径本来就是 async 的），要么——如果词法设计是有意为之——修正文档注释，说明分类器只在词法层面归一化 ..，在存在符号链接时可能与 loader 不一致。
见证——在未改动的 PR 代码上，用真实符号链接驱动真实 loader 与标签界面的探针输出：
CASE1 symlink <genRoot>/run.js -> <savedRoot>/deploy.js
loader: ACCEPTED, loaded content = the saved workflow
dialog: Generated workflow script: <genRoot>/run.js
CASE2 symlink <savedRoot>/toolgen.js -> <genRoot>/s-abc/throwaway.js
loader: ACCEPTED, loaded content = the generated script
dialog: Saved workflow: <savedRoot>/toolgen.js
应用建议的 realpath 分类后，两个标签都翻转为与实际加载内容一致，且 58 个 PR 测试全部仍然通过。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| it('classifies a ..-laced scriptPath by its normalized location', async () => { | ||
| const storage = new Storage( | ||
| path.join(os.tmpdir(), 'workflow-label-test'), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This new test inlines the same 4-line Storage fixture that the PR's own configWithStorage() helper (line 55) exists to provide, instead of calling it — the identical block now appears 5 times in this file (helper at :57 plus the label tests at :364, :386, :408 and :437). The helper is bypassed because of its return type: it returns only Config, while these tests need the storage handle to derive the expected roots — the file's own precedent configWithRegistry() returns { config, registry } for exactly this reason. The cost is concrete: any fixture change — the Storage constructor signature, the shared 'workflow-label-test' root, or isolating each test in its own tmp dir — must be made in five synchronized spots, and a miss is silent, because each test derives its assertion root from its own Storage instance and the label logic does no disk I/O, so every test stays green against a root the helper no longer shares.
function configWithStorage(): { config: Config; storage: Storage } {
const storage = new Storage(
path.join(os.tmpdir(), 'workflow-label-test'),
);
return { config: { storage } as unknown as Config, storage };
}
// in each label test:
const { config, storage } = configWithStorage();中文说明
[Suggestion] 这个新测试没有调用本 PR 自己添加的 configWithStorage() 辅助函数（第 55 行），而是内联构造了同一个 4 行 Storage 夹具——完全相同的代码块现在在本文件中出现 5 次（辅助函数 :57，加上 :364、:386、:408、:437 四个标签测试）。辅助函数被绕过的原因是它的返回类型：它只返回 Config，而这些测试需要 storage 句柄来推导期望的根目录——同文件中现成的先例 configWithRegistry() 正是为此返回 { config, registry }。代价是具体的：任何夹具改动——Storage 构造函数签名、共享的 'workflow-label-test' 根目录、或者把每个测试隔离到各自的 tmp 目录——都必须同步改五处，而且漏改不会报错：每个测试都从自己的 Storage 实例推导断言根目录，标签逻辑又不做任何磁盘 I/O，所以即使某个测试的夹具根目录与辅助函数不再一致，整套测试仍然全绿。
function configWithStorage(): { config: Config; storage: Storage } {
const storage = new Storage(
path.join(os.tmpdir(), 'workflow-label-test'),
);
return { config: { storage } as unknown as Config, storage };
}
// 在每个标签测试中：
const { config, storage } = configWithStorage();— qwen3.8-max via Qwen Code /review (v0.22.0)
| it('labels a nested generated-root scriptPath as a generated script', async () => { | ||
| const storage = new Storage( | ||
| path.join(os.tmpdir(), 'workflow-label-test'), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Same duplication as the sibling comment at line 406: this new test also inlines the 4-line Storage fixture the PR's own configWithStorage() helper exists to provide — the identical block now appears 5 times in this file (helper at :57 plus tests at :364, :386, :408 and :437). The helper returns only Config, so the label tests cannot reach the storage handle they derive expected roots from; returning { config, storage } like the adjacent configWithRegistry() lets every test use it. As written, a fixture change must touch five synchronized spots and a miss stays green — each test asserts against its own Storage instance's root, so the suite passes even when a test's fixture and the helper's no longer agree.
function configWithStorage(): { config: Config; storage: Storage } {
const storage = new Storage(
path.join(os.tmpdir(), 'workflow-label-test'),
);
return { config: { storage } as unknown as Config, storage };
}
// in each label test:
const { config, storage } = configWithStorage();中文说明
[Suggestion] 与 406 行的姊妹评论相同的重复：这个新测试同样内联了本 PR 自己的 configWithStorage() 辅助函数本该提供的 4 行 Storage 夹具——完全相同的代码块现在在本文件中出现 5 次（辅助函数 :57，加上 :364、:386、:408、:437 四个测试）。辅助函数只返回 Config，因此标签测试拿不到用于推导期望根目录的 storage 句柄；像相邻的 configWithRegistry() 一样返回 { config, storage } 即可让所有测试复用它。按现状写法，夹具改动必须同步五处，且漏改仍然全绿——每个测试都对自己的 Storage 实例根目录做断言，因此即使某个测试的夹具与辅助函数不再一致，测试套件依旧通过。
function configWithStorage(): { config: Config; storage: Storage } {
const storage = new Storage(
path.join(os.tmpdir(), 'workflow-label-test'),
);
return { config: { storage } as unknown as Config, storage };
}
// 在每个标签测试中：
const { config, storage } = configWithStorage();— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9987Commit: Feedback points and dispositionsrc:3857387219 — R4-1 follow-up (Suggestion,
|
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): workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21621, 1702, 24736, 1659, 601, 4237, 630 passed; 784 passed — this review observed 21621, 1702, 24736, 1659, 601, 4237, 630 passed.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/workflow/workflow.ts:701 — [review] canonical classifier hand-copies the loader's per-root canonicalization — next root-policy edit must patch both files or the consent label divergespackages/core/src/tools/workflow/workflow.ts:312 — [probe] grant axis does not branch on isGeneratedScriptPath — generated scripts are offered always-allow and a persistable exact-path rule
中文说明
Test Plan（非阻断）：workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21621, 1702, 24736, 1659, 601, 4237, 630 passed; 784 passed — this review observed 21621, 1702, 24736, 1659, 601, 4237, 630 passed。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no action takenThe only feedback item this round is the failed check Test (windows-latest, Node 22.x). There were no inline comments, no issue-level comments, and the review posted 0 findings (its two remaining observations are explicitly deferred under the convergence posture, recorded and not requested). After evidence-based investigation, the Windows failure is not caused by this PR — it is a broken Windows test lane on Evidence: the failing tests are outside this PR's footprintThe failing run (check run
This PR's diff ( Evidence: the same failure reproduces on unrelated branchesTwo unrelated PRs from the same window fail the Windows lane with the identical signature:
The failing set also shifts between runs (earlier heads of this branch failed on Evidence: this PR's own tests pass on WindowsThe failing run has no failure annotations for No base update is available
Local verification (Linux)
Conclusion: the failed check is base-induced Windows-lane breakage reproduced across unrelated branches; this PR's own tests are green on both platforms. The correct action is no action on this branch. 中文说明Autofix 本轮：未采取任何改动本轮唯一的反馈项是失败检查 Test (windows-latest, Node 22.x)。没有行内评论，没有 issue 级评论，审查本身发布了 0 条发现（其余两条观察项已在收敛姿态下明确延后——仅记录、本轮不要求处理）。经基于证据的调查，该 Windows 失败并非本 PR 引起——而是 证据：失败的测试不在本 PR 的影响范围内失败运行（check run
本 PR 的 diff（ 证据：同样的失败在不相关分支上复现同一时间窗内的两个不相关 PR 以完全相同的特征在 Windows 通道上失败：
失败集合在不同运行之间还会漂移（按第 5 轮第二意见审查所述，本分支更早的 head 上失败的是 证据：本 PR 自身的测试在 Windows 上通过失败运行中， 没有可用的 base 更新
本地验证（Linux）
结论：失败检查是 main 引起的 Windows 通道损坏，已在不相关分支上复现；本 PR 自身的测试在两个平台上均为绿色。对本分支的正确操作就是不操作。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 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.
No blocking issues. LGTM! ✅
Not explored to full depth (tool budget reached): "agent 6b": none — all checks above ran to completion..
Test Plan (not a blocker): workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21621, 1702, 1659, 601, 4235, 630 passed; 784 passed — this review observed 21621, 1702, 1659, 601, 4235, 630 passed.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/workflow/workflow.ts:687 — [review] canonical classifier duplicates the loader's root-normalization policy — next root-policy edit must patch both files or the consent label divergespackages/core/src/tools/workflow/workflow.ts:928 — [review] generated-root contract prose in the tool description is anchored by no test despite the file's prose-anchoring conventionpackages/core/src/agents/runtime/workflow-saved.test.ts:298 — [review] subprocess contract test mirrors the Config registration instead of exercising it — the registered value stays unpinned
中文说明
无阻断问题。LGTM！✅
未探索到全部深度（达到工具调用预算）："agent 6b"：none — all checks above ran to completion.。
Test Plan（非阻断）：workflows/generated-evil/x.js — no such file or directory; 194 passed — this review observed 21621, 1702, 1659, 601, 4235, 630 passed; 784 passed — this review observed 21621, 1702, 1659, 601, 4235, 630 passed。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
Released in v0.22.2. |
…QwenLM#10119) * feat(review): emit the Step 3A fan-out as a generated workflow script `qwen review emit-workflow` builds the roster the same way `agent-prompt --roster` does — same plan, same `buildLaunch`, same briefs, same prompts, same recorded delivery evidence — and writes those prompts into a runnable workflow script instead of printing thirteen blocks for the orchestrator to copy. The script lives under the generated-scripts root the Workflow loader trusts since QwenLM#9987 (`$QWEN_CODE_PROJECT_DIR/workflows/ generated/review/<session>/`), so it is never a slash command and needs no cleanup sweep. The generated file is a fixed body plus three literals — the roster, the worktree pin, the subagent type. No logic is generated, only data, and the tests execute the generator's real output. A territory fan-out (Step 3B) and an unsized plan are refused before anything is written, because a workflow returns every agent through one tool result and a roster that grows with the diff is silently truncated there. Nothing routes through the command yet: the skill still builds its roster with `agent-prompt --roster`. Routing is its own change. Part of QwenLM#8769. Claude-Session: https://claude.ai/code/session_017cUwuTey4APA8wAyAM6ScS * test(review): mirror the sandbox runtime in fan-out script tests (QwenLM#10119) Address review feedback on the emit-workflow PR: - Run the generated fan-out script in a vm context that mirrors the workflow sandbox's execution shape: the meta block is stripped instead of executed, the body is wrapped in the runtime's strict-mode async IIFE, only the sandbox globals are bound, and the agent stub applies the runtime's option gates. - Extend the determinism guard to the sandbox's full Date surface (Date.parse, Date.UTC, bare Date calls). - Exercise the failed-write half of the temp-and-rename cleanup. - Cover the handler-level --rules happy path end to end. * fix(review): harden the generated fan-out path and fail closed (QwenLM#10119) Address the four review blockers on the emit-workflow PR: - Share the loader's canonical-containment policy on the write side: refuse a symlinked directory from the generated root down to the session dir, and prove the canonical session dir stays under the canonical root, before any brief, prompt record, or script is written. - Keep colliding sanitized session ids apart by appending a digest of the RAW session id to the readable prefix, so two concurrent sessions can never select the same script target for the same plan. - Canonicalize an existing plan path with realpath before hashing it, so one plan keeps one script name under divergent spellings of the same file (macOS /var vs /private/var, or a link). - Fail the fan-out whenever any required agent delivered nothing, instead of returning a shortened delivered list; a missing role is a failed step, not a shorter finding set. * test(review): canonicalize fixtures and pin the review-dir symlink guard (QwenLM#10119) * test(review): pin the emit-workflow cannot-read-the-plan guard (QwenLM#10119) * test(review): pin emit-workflow dispatch guidance and clean-tree silence (QwenLM#10119) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): drop the duplicated telemetry-swap mock property The `Merge branch 'main'` in 1f670a1 brought main's TS1117 in with it: `client.telemetrySwap.test.ts` declares `getToolRegistry` twice in the same object literal, which fails `tsc` and kills `packages/core`'s build before a single test runs — the Test job dies in "Install dependencies". Not this branch's doing. Two main commits added the property independently and neither saw the other: 032b907 feat(serve): backfill session PR bindings ... (QwenLM#9729) 8241905 test(core): give the telemetry-swap client mock a getToolRegistry (QwenLM#10220) `upstream/main` at 053f17b still carries both — checking that exact file out here and running `tsc --noEmit -p packages/core` reproduces `client.telemetrySwap.test.ts(103,5): error TS1117` verbatim, so main is red on its own and every branch that merges it inherits this. Keeps QwenLM#10220's copy — it was added for this purpose and carries the explanation — and drops QwenLM#9729's incidental one. main needs the same removal; this only unblocks the branch. Verified: `tsc --noEmit -p packages/core` clean, `client.telemetrySwap.test.ts` 10 passed. Claude-Session: https://claude.ai/code/session_01M7z4PccYfDPyyfg3oGr8V1 * fix(review): hermetic probe fixtures and honest fan-out failure messages (QwenLM#10119) The deterministic gate's `--changed` run collects test-efficacy.test.ts through this PR's lib/paths.ts change, and its skip-worktree guard test died on a persistent runner: the fixture's raw git calls inherited an ambient discovery redirect (GIT_INDEX_FILE reproduces the exact failure) while the guard reads a sanitized env, so the bit landed in another index and the refusal never fired. Run every fixture git call with the same sanitized env the guard uses. Address the maintainer verification of this PR: - A fan-out where EVERY agent failed prescribed re-running emit-workflow, which regenerates the identical script with the identical baked-in pin — a loop. Name the dispatch instead. - The territory refusal claimed results are "silently truncated away"; the scheduler persists large results and hands the model a pointer. Restate the real bound: the run's wall-clock caps and the fail-closed guard a per-chunk roster makes near-certain. - A refused plan no longer leaves the empty session directory (blocker check moved ahead of the mkdir). - The roster-key mismatch guard gains its missing test. Also drop the duplicate getToolRegistry the merge of main brought into client.telemetrySwap.test.ts (TS1117 broke `npm run build`; the two entries were byte-identical). * test(review): isolate test-efficacy fixtures from host git config (QwenLM#10119) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
5 participants
What this PR does
Adds a third trusted root for loading a workflow by path:
<projectDir>/workflows/generated, beside the run snapshots and resume journals in the runtime directory.Workflow({scriptPath})and the in-scriptworkflow({scriptPath})accept any file that resolves inside it, under the same realpath boundary check and symlinked-root refusal the saved-workflow directories already get. Nothing in it is a saved workflow: it is never enumerated for/<name>slash commands, andworkflow('<name>')cannot reach it. Layout below the root is the writer's choice (per-session subdirectories are fine); the loader trusts the whole subtree. The Workflow tool's description also stops claiming thatscriptPathaccepts a path anywhere — it never did — and now names the three roots that are accepted.Why it's needed
The loader only accepts scripts inside
.qwen/workflowsor~/.qwen/workflows, and every.jsfile in those two directories is simultaneously a slash command in the user's session. So a tool that generates a workflow script for one run has no legitimate place to put it. The first attempt at emitting/review's fan-out as a workflow (#8943) ran straight into this: the script had to be written into.qwen/workflows, which turned every review into a permanent/qwen-review-<digest>command, which in turn required digest naming, a cleanup sweep, symlink guards, and writer/sweeper parity tests — several hundred lines and a recurring cluster of review findings, all to work around a missing runtime capability. A generated-scripts root that is loadable but not addressable removes the entire category: the emitting tool writes there, the model's call carries one path, and nothing leaks into the command namespace or the project tree.Reviewer Test Plan
How to verify
<projectDir>/workflows/generated/anything.js(the project dir is~/.qwen/projects/<sanitized-cwd>by default, or underQWEN_RUNTIME_DIR; a subprocess reaches it as$QWEN_CODE_PROJECT_DIR/workflows/generated), then call the Workflow tool with thatscriptPath. Expected: it runs. A nested path such asgenerated/s-<session>/fanout.jsruns too.workflow('anything')from inside another workflow reports "no workflow with that name".<projectDir>/workflows/generated-evil/x.js), a file inside the root that is a symlink to somewhere outside, and any file behind agenerateddirectory that is itself a symlink.cd packages/core && npx vitest run src/agents/runtime/workflow-saved.test.ts src/config/storage.test.ts src/tools/workflow/workflow.test.ts— 3 files, 201 passed (6 new in the generated-root block, 1 new storage path test).src/agents/runtime/as a whole: 16 files passed, 1 skipped; 765 passed, 6 skipped. (Both re-measured on headff86990.)Evidence (Before & After)
N/A — no user-visible change; behavior is pinned by the unit tests above.
Tested on
Environment (optional)
Unit tests only (vitest, Node 22).
Risk & Scope
Workflow(scriptPath:<path>)still pre-approves one exact path).Linked Issues
Part of #8769 — the runtime prerequisite that lets a CLI-generated review fan-out be loaded without occupying the user's saved-workflow namespace. Follows #8971 and #8972.
中文说明
本 PR 做了什么
为按路径加载 workflow 增加第三个受信根目录：
<projectDir>/workflows/generated，与 runtime 目录中的 run 快照和 resume journal 并列。Workflow({scriptPath})和脚本内的workflow({scriptPath})接受解析到该目录内的任何文件，并沿用保存目录已有的 realpath 边界检查与软链根目录拒绝逻辑。其中的文件都不是"保存的 workflow"：不会被枚举为/<name>slash command，workflow('<name>')也无法触达。根目录以下的布局由写入方决定（按 session 分子目录也可以），loader 信任整棵子树。Workflow 工具描述也不再声称scriptPath可以是任意路径——它从来不是——而是列出实际接受的三个根目录。为什么需要
loader 只接受
.qwen/workflows或~/.qwen/workflows内的脚本，而这两个目录里的每个.js同时也是用户会话中的 slash command。因此，一个为单次运行生成 workflow 脚本的工具没有合法的落点。第一次尝试把/review的扇出以 workflow 形式发出（#8943）正是撞上了这一点：脚本只能写进.qwen/workflows，于是每次 review 都变成一个永久的/qwen-review-<digest>命令，进而需要 digest 命名、清理扫描、软链守卫和写入/清理双方的对齐测试——几百行代码加上一簇反复出现的评审 finding，全是在绕一个 runtime 缺失的能力。一个"可加载但不可寻址"的生成脚本根目录消灭了整个类别：发出脚本的工具写到那里，模型的调用只带一个路径，不会有任何东西泄漏进命令命名空间或项目树。评审验证计划
如何验证
<projectDir>/workflows/generated/anything.js放一个脚本（project dir 默认是~/.qwen/projects/<sanitized-cwd>，或在QWEN_RUNTIME_DIR下；子进程可通过$QWEN_CODE_PROJECT_DIR/workflows/generated定位），然后以该scriptPath调用 Workflow 工具。预期：正常运行。嵌套路径如generated/s-<session>/fanout.js同样可运行。workflow('anything')报 "no workflow with that name"。<projectDir>/workflows/generated-evil/x.js）、根目录内软链到外部的文件、以及generated目录本身是软链时其后的任何文件。cd packages/core && npx vitest run src/agents/runtime/workflow-saved.test.ts src/config/storage.test.ts src/tools/workflow/workflow.test.ts—— 3 个文件、201 通过（generated-root 块新增 6 个，storage 路径测试新增 1 个）。整个src/agents/runtime/：16 个文件通过、1 个跳过，765 通过、6 跳过。（均在 headff86990上重新实测。）证据（前后对比）
N/A —— 无用户可见改动；行为由上述单元测试钉住。
测试平台
环境（可选）
仅单元测试（vitest，Node 22）。
风险与范围
Workflow(scriptPath:<path>)仍只预批准一个精确路径）。关联 Issue
Part of #8769 —— 让 CLI 生成的 review 扇出脚本可以被加载而不占用用户保存的 workflow 命名空间的 runtime 前置。承接 #8971 与 #8972。
https://claude.ai/code/session_017cUwuTey4APA8wAyAM6ScS