Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions packages/core/src/tools/workflow/workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import type { Config } from '../../config/config.js';
import { ToolNames, ToolDisplayNames } from '../tool-names.js';
import { WorkflowRunRegistry } from '../../agents/workflow-run-registry.js';
import { WorkflowJournal } from '../../agents/runtime/workflow-journal.js';
import {
DEFAULT_MAX_AGENTS_PER_RUN,
MAX_WORKFLOW_AGENTS_ENV,
MAX_WORKFLOW_CONCURRENCY_ENV,
} from '../../agents/runtime/workflow-orchestrator.js';
import { Storage } from '../../config/storage.js';

function fakeConfig(): Config {
Expand Down Expand Up @@ -54,6 +59,89 @@ describe('WorkflowTool', () => {
);
});

// The description is what makes the model pick pipeline() over a barrier
// and verify a finding before reporting it. A refactor that drops the
// policy prose leaves a runtime nobody drives well, and no other test
// would notice — so anchor the load-bearing claims.
it('description carries both the runtime facts and the orchestration policy', () => {
const { description } = new WorkflowTool(fakeConfig());
Comment on lines +66 to +67

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-1: The pinning test's anchors leave policy sections unguarded. The test exists — per its own comment — to catch "A refactor that drops the policy prose", but deleting the "What a workflow is for" or "Scout first, then orchestrate" section (including "Common single-phase shapes") from WORKFLOW_TOOL_DESCRIPTION keeps all 10 assertions green (verified by mutation probe). The linked issue's six-item scope includes purpose framing, hybrid scouting, and reusable shapes — none of them is anchored. — Failure scenario: a future refactor deletes those policy paragraphs → every anchor stays intact and the suite passes green, shipping exactly the regression class this test claims to catch for three of the issue's six content areas.

Add one anchor per uncovered section:

expect(description).toMatch(/Parallelism on its own is not a reason/);
expect(description).toMatch(/only before the orchestration step/);
expect(description).toMatch(/Common single-phase shapes/);
中文说明

[Suggestion] 锚定测试的锚点没有覆盖全部策略段落。该测试按其自身注释的说法,是为了捕获"删掉策略 prose 的重构",但经变异探针验证:从 WORKFLOW_TOOL_DESCRIPTION 中删除 "What a workflow is for" 或 "Scout first, then orchestrate"(含 "Common single-phase shapes")段落后,全部 10 条断言仍然为绿。关联 issue 的六点范围包含目的定位、混合侦察与可复用形状——这三项均未被锚定。失败场景:未来某次重构删掉这些策略段落 → 所有锚点原样存活、测试套件全绿,恰好把该测试声称要捕获的回归类型放行(涉及 issue 六点中的三点)。

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

// Every env knob the description names is anchored. The two that the
// orchestrator exports are anchored *through the exported constant*, so
// a rename on the runtime side fails here too — a hardcoded literal
// would only have caught a description-side typo, and the model would
// go on telling users to set a variable nothing reads.
// `QWEN_CODE_MAX_WORKFLOW_SECONDS` has no exported constant
// (`workflow-sandbox.ts` reads it inline), so it stays a literal.
for (const anchor of [
'min(16, cpus-2)',
MAX_WORKFLOW_AGENTS_ENV,
MAX_WORKFLOW_CONCURRENCY_ENV,
'QWEN_CODE_MAX_WORKFLOW_SECONDS',
'resumeFromRunId',
'/workflows',
'node:vm sandbox',
]) {
expect(description).toContain(anchor);
}
// One anchor per policy section — dropping any whole section has to
// turn this test red, which is the regression it exists to catch.
expect(description).toMatch(/Parallelism on its own is not a reason/);
expect(description).toMatch(/only before the orchestration step/);
expect(description).toMatch(/Common single-phase shapes/);
expect(description).toMatch(/Default to `pipeline\(\)`/);
expect(description).toMatch(/A barrier is right only when/);
expect(description).toMatch(/refute/);
expect(description).toMatch(/against everything already seen/);
expect(description).toMatch(/log\(\)` what was dropped/);
// Limits the model has to plan around rather than discover from a
// mid-run failure — the numbers themselves, not just the knob names.
// Anchored *through* the exported constant rather than as a literal:
// the description interpolates `DEFAULT_MAX_AGENTS_PER_RUN`, so this
// tracks a raised cap automatically, and a regression that pastes the
// number back in as prose goes red the next time the constant moves.
expect(description).toContain(
`up to ${DEFAULT_MAX_AGENTS_PER_RUN} agents total`,
);
// `DEFAULT_MAX_WALL_CLOCK_MS` is private to `workflow-sandbox.ts`, so
// this one is still a hand-synced literal on both sides.
expect(description).toMatch(/30-minute wall-clock cap/);
expect(description).toMatch(/nests one level only/);
Comment on lines +96 to +108

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-2: The numeric caps the model plans around — "up to 1000 agents total" and the "30-minute wall-clock cap" — are not anchored by any assertion, even though this comment names exactly those planning limits and the sibling min(16, cpus-2) formula IS anchored. — Failure scenario: a future PR raises DEFAULT_MAX_AGENTS_PER_RUN or DEFAULT_MAX_WALL_CLOCK_MS and forgets WORKFLOW_TOOL_DESCRIPTION → all 17 anchors stay green while the model sizes fan-outs against stale caps, discovering the real one from a mid-run rejection. Probe-verified: mutating the description's 1000500 and 30-minute15-minute leaves the test green; adding the two anchors below flips the same mutation red.

Suggested change
// Limits the model has to plan around rather than discover from a
// mid-run failure.
expect(description).toMatch(/nests one level only/);
// Limits the model has to plan around rather than discover from a
// mid-run failure.
expect(description).toMatch(/up to 1000 agents total/);
expect(description).toMatch(/30-minute wall-clock cap/);
expect(description).toMatch(/nests one level only/);
中文说明

[Suggestion] 模型制定计划所依据的数值上限——"最多 1000 个 agent"与"30 分钟墙钟上限"——没有任何断言锚定,尽管这段注释指名的正是这些规划限制,且同段的 min(16, cpus-2) 公式已被锚定。失败场景:未来某个 PR 提高 DEFAULT_MAX_AGENTS_PER_RUNDEFAULT_MAX_WALL_CLOCK_MS 却忘记更新 WORKFLOW_TOOL_DESCRIPTION → 所有 17 个锚点保持绿色,而模型按过时的上限规划扇出,直到运行中途被拒绝才发现真实上限。探针验证:将描述中的 100050030-minute15-minute 变异后测试仍绿;加入上面两个锚点后同一变异即转红。

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

expect(description).toMatch(/read `budget\.total`/);
// The `/workflows` capability list is the one part of the description
// that trails the runtime: #8320 added cooperative pause/resume to the
// dialog while this branch was moving the description into a constant,
// and the base merge conflicted exactly here. Nothing else asserts the
// control set, so dropping one on the next merge would be silent.
expect(description).toMatch(/cooperative pause\/resume/);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The /workflows dialog capability list is under-anchored in two probe-proven shapes: (1) the loop anchor toContain('/workflows') above is permanently masked by the saved-workflow path strings — both <projectRoot>/.qwen/workflows and ~/.qwen/workflows contain /workflows — and (2) of the four advertised capabilities, "(live phase tree, token usage, cooperative pause/resume, cancel)", only cooperative pause/resume is anchored. — Failure scenario: probed at this commit — deleting only the /workflows dialog reference from the sentence leaves 41/41 green (the path strings satisfy the masked anchor), and deleting , cancel from the parenthetical also leaves 41/41 green. Either silent drop removes the model's only description-side pointer to the dialog or its controls — exactly the #8320-style merge drop the comment above says this anchor exists to catch.

Suggested change
expect(description).toMatch(/cooperative pause\/resume/);
expect(description).toMatch(/the `\/workflows` dialog \(live phase tree, token usage, cooperative pause\/resume, cancel\)/);

(one unit anchor pins the dialog reference plus all four capabilities; the masked '/workflows' loop entry can then be dropped)

中文说明

/workflows 对话框能力清单的锚定不足,且两种形态都已用探针证实:(1) 上面循环里的 toContain('/workflows') 锚点被 saved-workflow 路径字符串永久遮蔽——<projectRoot>/.qwen/workflows~/.qwen/workflows 都包含 /workflows 子串;(2) 描述宣称的四项能力 "(live phase tree, token usage, cooperative pause/resume, cancel)" 中只有 cooperative pause/resume 被锚定。失败场景:已在被审 commit 上用探针验证——只删掉句中的 /workflows 对话框引用,41/41 依旧全绿(路径字符串能满足被遮蔽的锚点);删掉括号里的 , cancel 同样全绿。任何一种静默删除都会抹掉模型在描述侧指向该对话框或其控件的唯一线索——正是上方注释声称这个锚点要拦截的 #8320 式合并丢失。(建议块用一条整体锚点同时钉住对话框引用与全部四项能力;之后可以删掉被遮蔽的 '/workflows' 循环项。)

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

// #8690 asked the text to speak this project's own vocabulary. Without
// a location, "runs a saved workflow" leaves the model no way to reach
// one: `workflow('<name>')` is a blind guess and `scriptPath` wants an
// absolute path it cannot construct.
expect(description).toContain('.qwen/workflows');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] This anchor is masked by the user-scope path: ~/.qwen/workflows contains the substring .qwen/workflows, so dropping or rewriting the project-scope clause <projectRoot>/.qwen/workflows while the user path remains leaves the anchor green — losing exactly the project-scope placement the comment above (issue #8690's "speak this project's own vocabulary" ask) says this anchor defends. — Failure scenario: a maintainer or the next base merge drops the project-scope mention while ~/.qwen/workflows remains → toContain('.qwen/workflows') still matches → the model loses the project-scope location (the primary one — project scope shadows user scope and is surfaced in the slash-command list first) and no test turns red.

Suggested change
expect(description).toContain('.qwen/workflows');
expect(description).toContain('<projectRoot>/.qwen/workflows');
expect(description).toContain('~/.qwen/workflows');

(the ~ form also pins the tilde, which .qwen/workflows never checks)

中文说明

这个锚点被用户作用域路径遮蔽:~/.qwen/workflows 本身就包含 .qwen/workflows 子串,因此只要用户侧路径还在,删掉或改写项目作用域的 <projectRoot>/.qwen/workflows 子句,锚点依旧为绿——丢掉的恰恰是上方注释(issue #8690 "使用本项目自己的词汇"的要求)声称这个锚点要守护的项目作用域位置。失败场景:维护者或下一次 base 合并删掉了项目作用域的提及而 ~/.qwen/workflows 仍在 → toContain('.qwen/workflows') 仍然匹配 → 模型失去项目作用域的位置信息(那是主要位置——项目作用域遮蔽用户作用域,且在斜杠命令列表中最先呈现),同时没有任何测试转红。(~ 的写法同时还钉住了波浪号,.qwen/workflows 永远检查不到它。)

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

});

// The tool description is not the only model-visible copy of the caps —
// the `script` parameter description states them a second time, and a
// model reading one tool call sees both. Anchoring only the tool
// description lets a maintainer raise a cap, watch the test above go
// green again, and stop while `script` still advertises the old number.
it('script parameter description states the same caps as the tool description', () => {
const tool = new WorkflowTool(fakeConfig());
const schema = tool.schema.parametersJsonSchema as {
properties: { script: { description: string } };
};
const scriptDescription = schema.properties.script.description;
expect(scriptDescription).toContain(
`At most ${DEFAULT_MAX_AGENTS_PER_RUN} agent() calls per run`,
);
expect(scriptDescription).toContain(MAX_WORKFLOW_AGENTS_ENV);
expect(scriptDescription).toContain(MAX_WORKFLOW_CONCURRENCY_ENV);
// Both halves must agree on the agent cap, whatever it is.
expect(tool.description).toContain(
`${DEFAULT_MAX_AGENTS_PER_RUN} agents total`,
);
});

it('rejects build() when script is missing', () => {
const tool = new WorkflowTool(fakeConfig());
expect(() => tool.build({} as never)).toThrow(/script/);
Expand Down
84 changes: 64 additions & 20 deletions packages/core/src/tools/workflow/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ import { ToolNames, ToolDisplayNames } from '../tool-names.js';
import { ToolErrorType } from '../tool-error.js';
import type { Config } from '../../config/config.js';
import type { WorkflowAgentDispatch } from '../../agents/runtime/workflow-orchestrator.js';
import {
DEFAULT_MAX_AGENTS_PER_RUN,
MAX_WORKFLOW_AGENTS_ENV,
MAX_WORKFLOW_CONCURRENCY_ENV,
} from '../../agents/runtime/workflow-orchestrator.js';
import { MAX_TOKENS_PER_WORKFLOW_ENV } from '../../agents/runtime/workflow-budget.js';
import {
WorkflowRunner,
Expand Down Expand Up @@ -106,7 +111,7 @@ const WORKFLOW_PARAM_SCHEMA = {
'Concurrency: `parallel([() => agent(...), ...])` runs thunks ' +
'through a shared per-run window (default ' +
'`max(1, min(16, cpus-2))` agents in flight; override via ' +
'`QWEN_CODE_MAX_WORKFLOW_CONCURRENCY`) and resolves to a ' +
`\`${MAX_WORKFLOW_CONCURRENCY_ENV}\`) and resolves to a ` +
'position-aligned array — a thunk that throws, or resolves to a ' +
'non-JSON-serializable value, becomes `null` at its index ' +
'(errors-as-data); parallel() itself rejects only on invalid ' +
Expand All @@ -115,8 +120,9 @@ const WORKFLOW_PARAM_SCHEMA = {
'that throws, returns `null`, or returns a non-JSON-serializable ' +
'value drops that item to `null`. Pass ' +
'THUNKS to parallel, not eager calls: `parallel([() => agent(...)])`, ' +
'not `parallel([agent(...)])`. At most 1000 agent() calls per run ' +
'(override via `QWEN_CODE_MAX_WORKFLOW_AGENTS`). ' +
'not `parallel([agent(...)])`. At most ' +
`${DEFAULT_MAX_AGENTS_PER_RUN} agent() calls per run ` +
`(override via \`${MAX_WORKFLOW_AGENTS_ENV}\`). ` +
'`Date.now()` and `Math.random()` both throw — workflow scripts ' +
'must be deterministic for resume. ' +
'`export const meta = {...}` declarations are stripped before execution.',
Expand Down Expand Up @@ -509,6 +515,60 @@ function safeStringifyDisplayPayload(payload: unknown): string {
}
}

/**
* The tool description the model reads before deciding to orchestrate. The
* capability half (globals, limits, per-call options) is only half the job:
* without the policy half, the same runtime reliably produces the naive
* shape — everything through one `parallel()` barrier, first answer taken at
* face value. The prose below is therefore load-bearing, not documentation.
* `script`'s own description carries the exact authoring contract (error
* strings, serialization rules). The agent cap and the two env knobs are
* interpolated from the orchestrator's exported constants
* (`DEFAULT_MAX_AGENTS_PER_RUN`, `MAX_WORKFLOW_AGENTS_ENV`,
* `MAX_WORKFLOW_CONCURRENCY_ENV`) in both halves, so raising a cap moves
* every model-visible copy at once — there is no prose to hand-sync.
* The wall-clock cap is the one exception: `DEFAULT_MAX_WALL_CLOCK_MS` is
Comment on lines +529 to +530

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The "there is no prose to hand-sync … the wall-clock cap is the one exception" claim is inaccurate: the default concurrency window max(1, min(16, cpus-2)) is also hand-synced prose in BOTH model-visible descriptions — resolveConcurrencyLimit computes it inline (workflow-orchestrator.ts) and no constant is exported, so the tool description and the script parameter description each carry the formula as prose. — Failure scenario: a maintainer widens the default window (16 → 32) → both descriptions keep advertising min(16, cpus-2) and nothing bridges the runtime to either description: the 'min(16, cpus-2)' anchor in the pinning test only trips if the tool-description prose is edited, and the parity test anchors the agent cap and env knobs in the script description but not the formula — while this comment actively assures the reader that the wall-clock literal is the only one needing a companion edit.

Suggested change
* every model-visible copy at once there is no prose to hand-sync.
* The wall-clock cap is the one exception: `DEFAULT_MAX_WALL_CLOCK_MS` is
* every model-visible copy at once there is no prose to hand-sync.
* The wall-clock cap and the `max(1, min(16, cpus-2))` default window are the exceptions: `DEFAULT_MAX_WALL_CLOCK_MS` is
中文说明

"无需手工同步的文案……墙钟上限是唯一的例外"这一说法不准确:默认并发窗口 max(1, min(16, cpus-2)) 同样是两份模型可见描述中的手工同步文案——resolveConcurrencyLimit 行内计算该值(workflow-orchestrator.ts),没有导出常量,因此工具描述与 script 参数描述各自以文案形式携带该公式。失败场景:维护者把默认窗口放宽(16 → 32)→ 两份描述继续宣称 min(16, cpus-2),且没有任何东西把运行时与这两份描述关联起来:钉定测试里的 'min(16, cpus-2)' 锚点只在工具描述文案被编辑时才会转红,而对等测试只钉定了 script 描述里的 agent 上限与环境旋钮、没有钉定该公式——而这条注释却在主动向读者保证墙钟字面量是唯一需要配套修改的地方。

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

* private to `workflow-sandbox.ts`, so "30-minute" is still a literal here
* and has to be edited alongside it. The output-token budget and the
* one-level `workflow()` nesting limit appear ONLY here, so this text is
* their model-visible source of truth.
*/
const WORKFLOW_TOOL_DESCRIPTION = `Execute a workflow script that orchestrates subagents deterministically.

**What a workflow is for**

Reach for one to be comprehensive (decompose the work and cover every part in parallel), to be confident (independent perspectives and adversarial checks before an answer is committed to), or to take on scale a single context cannot hold — migrations, audits, broad sweeps. The script is where that structure is encoded: what fans out, what verifies, what synthesizes. Parallelism on its own is not a reason; work that is already one short sequence of edits belongs in the main loop.

**Runtime** — see the \`script\` parameter for the detailed authoring contract.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-2: The description newly advertises the workflow(nameOrRef, args?) and budget globals while pointing at the script parameter for "the detailed authoring contract" — but that contract documents neither global. The hard single-level nesting limit (the sandbox throws "workflow() nesting is limited to a single level") and budget's total/spent()/remaining() semantics appear nowhere model-visible. — Failure scenario: an author composing two levels of workflow() nesting — the natural reading of an advertised primitive — discovers the throwing stub only at runtime → the run has already burned agents/tokens on everything before the failing call.

For example, add to the Runtime paragraph: "workflow(nameOrRef, args?) runs a saved workflow sharing this run's caps; nesting is single-level (a workflow invoked via workflow() cannot call workflow() itself)", and/or document both globals in the script parameter description.

中文说明

[Suggestion] 描述新近宣传了 workflow(nameOrRef, args?)budget 全局,并指向 script 参数的"详细编写契约"——但该契约对这两个全局只字未提:单层嵌套的硬性上限(沙箱会抛出 "workflow() nesting is limited to a single level")以及 budgettotal/spent()/remaining() 语义在任何模型可见的文本中都不存在。失败场景:作者组合两层 workflow() 嵌套——对被宣传原语的自然读法——只在运行时才撞到 throwing stub → 此时本次运行已在失败调用之前烧掉了 agents/tokens。

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


\`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. 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' })\` covers structured-output contracts, declarative-agent selection, model override, and git-worktree-isolated subagents. \`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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The description states that scriptPath "takes an absolute path to a script anywhere", but the runtime refuses any scriptPath outside the two saved-workflow directories (<projectRoot>/.qwen/workflows and ~/.qwen/workflows). Probe-verified at this commit through the real tool path: a scriptPath pointing at a file elsewhere throws refusing to load a workflow file outside the saved-workflow directories (the realpath boundary check in workflow-saved.ts). Enforcement is stricter than advertised — fail-safe direction, so this is a description defect, not a missing guard — but the clause also contradicts the scriptPath parameter's own schema description ("Absolute path to a saved workflow .js file"), so the model reads two incompatible statements in one tool declaration. — Failure scenario: a model trusts the description and passes a path like ~/scratch/my-flow.js as scriptPath → the call fails every time, in a way the description says cannot happen → the model retries or improvises instead of telling the user to place the file under one of the two directories or inline it via script.

// in WORKFLOW_TOOL_DESCRIPTION (Runtime paragraph):
- while `scriptPath` takes an absolute path to a script anywhere.
+ while `scriptPath` takes an absolute path to a saved-workflow file
+ inside one of them (paths outside are refused).

(the matching new test comment in workflow.test.ts — "scriptPath wants an absolute path it cannot construct" — wants the same adjustment)

中文说明

描述声称 scriptPath "接受指向任意脚本的绝对路径",但运行时拒绝任何位于两个 saved-workflow 目录(<projectRoot>/.qwen/workflows~/.qwen/workflows)之外的 scriptPath。已在被审 commit 上通过真实工具路径用探针验证:指向其他位置的 scriptPath 会抛出 refusing to load a workflow file outside the saved-workflow directoriesworkflow-saved.ts 中的 realpath 边界检查)。实际执行比描述更严格——方向是 fail-safe 的,因此这是描述缺陷而非缺少防护——但该子句同时与 scriptPath 参数自身的 schema 描述("Absolute path to a saved workflow .js file")矛盾,模型会在同一次工具声明里读到两种互不相容的说法。失败场景:模型相信描述,把 ~/scratch/my-flow.js 这类路径作为 scriptPath 传入 → 调用每次都按描述声称"不可能发生"的方式失败 → 模型反复重试或改走野路子,而不是告诉用户把文件放到上述两个目录之一、或用 script 内联。(workflow.test.ts 中新增的那句注释——"scriptPath wants an absolute path it cannot construct"——也建议一并调整。)

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The parenthetical pins slash-command surfacing to project scope only — "(project scope, also surfaced as /<name> slash commands)" — but user-scope saved workflows are surfaced as /<name> slash commands too: SavedWorkflowLoader.loadCommands maps every entry from listSavedWorkflows (both scopes, project shadowing same-named user entries) to a command with no scope filter, and its own test feeds a user-scope entry and asserts it becomes a command. — Failure scenario: a model reading this description concludes a user-scope workflow is not reachable as /<name> → it avoids recommending that path, or tells the user their user-scope workflow has no slash command when it does. Impact is bounded because workflow('<name>') resolution against both directories is documented in the same sentence.

// in WORKFLOW_TOOL_DESCRIPTION (Runtime paragraph):
- `<projectRoot>/.qwen/workflows` (project scope, also surfaced as
- `/<name>` slash commands) or `~/.qwen/workflows` (user scope, ...)
+ `<projectRoot>/.qwen/workflows` (project scope) or `~/.qwen/workflows`
+ (user scope, lower precedence when both define the same name); both
+ are surfaced as `/<name>` slash commands
中文说明

这个括号注释把斜杠命令的暴露限定在了项目作用域——"(project scope, also surfaced as /<name> slash commands)"——但用户作用域的 saved workflow 同样会暴露为 /<name> 斜杠命令:SavedWorkflowLoader.loadCommands 会把 listSavedWorkflows(两个作用域,项目侧遮蔽同名的用户侧条目)返回的每个条目都映射为命令,没有任何作用域过滤,它自己的测试就喂入过一个用户作用域条目并断言其成为命令。失败场景:模型读到这段描述后断定用户作用域的 workflow 无法通过 /<name> 触达 → 于是避免推荐这条路径,或者在用户的 user-scope workflow 明明有斜杠命令时告诉他没有。影响有限,因为同一句话里也写了 workflow('<name>') 会在这两个目录里解析。

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


**Scout first, then orchestrate**

The strongest pattern is hybrid: discover the work list in the main loop (list the files, scope the diff, read the failing test), then hand that list to a workflow. You do not need to know the shape of the work before the task — only before the orchestration step. When the work has distinct phases, run several small workflows across turns and read each result before choosing the next, rather than authoring one large script that runs unattended.

Common single-phase shapes: understand (parallel readers over subsystems, merged into one map), design (independent approaches, judged, then synthesized), review (dimensions, find, verify each finding), research (broad sweep, deep read, synthesis), migrate (discover sites, transform each under \`isolation: 'worktree'\`, verify).

**Default to \`pipeline()\`**

\`pipeline()\` runs each item through every stage independently — item A can be in stage 3 while item B is still in stage 1 — so wall-clock is the slowest single chain. \`parallel()\` is a barrier: it waits for every thunk before anything moves on, so it costs the slowest item of every stage.

A barrier is right only when a stage genuinely needs cross-item context: deduplicating or merging across the full result set before expensive downstream work, exiting early when the total count is zero, or a prompt that compares one finding against all the others. It is not justified by needing to flatten, map, or filter between stages (do that inside a pipeline stage), by two stages being conceptually separate, or by the code reading more tidily. Smell test: \`parallel()\` → a pure transform → \`parallel()\` is a pipeline someone wrote with an unnecessary barrier. When in doubt, \`pipeline()\`.

**Verify before believing**

A subagent's answer is a claim, not a result. For findings that matter, spawn independent verifiers prompted to *refute*, and drop what a majority refutes. When a claim can be wrong in several different ways, give each verifier a distinct lens (correctness, security, performance, does it actually reproduce) — diversity catches what repetition cannot. For a wide solution space, generate several independent attempts, judge them in parallel, and synthesize from the winner while grafting the best ideas from the rest.

**Converge deliberately**

For discovery of unknown size, keep running finders until some number of consecutive rounds turn up nothing new; a fixed round count stops partway into the tail. Deduplicate each round against everything already seen, never against only what survived judging — otherwise rejected findings reappear every round and the loop never terminates. A closing pass that asks what is still missing (a search angle never run, a claim never verified, a file never read) usually produces the next round of real work.

**Report honestly**

Scale the fleet to what was actually asked: a quick check gets a few agents and one verification pass; an explicit request to be thorough or exhaustive earns a larger pool and a multi-vote adversarial round. Whenever a run bounds its own coverage — top-N, sampling, no retry — \`log()\` what was dropped. Silent truncation reads as full coverage, which is worse than a smaller honest result.

These shapes are a starting point, not a menu; compose the harness the task actually needs.`;

export class WorkflowTool extends BaseDeclarativeTool<
WorkflowParams,
ToolResult
Expand All @@ -520,23 +580,7 @@ export class WorkflowTool extends BaseDeclarativeTool<
super(
ToolNames.WORKFLOW,
ToolDisplayNames.WORKFLOW,
'Execute a workflow script that orchestrates subagents. ' +
'Supports `phase`, `log`, sequential `agent`, concurrent fan-out via ' +
'`parallel(thunks)` / `pipeline(items, ...stages)` (default ' +
'`max(1, min(16, cpus-2))` agents in flight per run, up to 1000 ' +
'agents total; both env-overridable), per-call `agent({ schema, ' +
"agentType, model, isolation: 'worktree' })` for structured-output " +
'contracts, declarative-agent selection, model override, and git-' +
'worktree-isolated subagents. Pass `resumeFromRunId` to resume a prior ' +
'run — agent() calls whose rolling prefix-hash matches the journal are ' +
'served from cache for the longest unchanged prefix. Runs are tracked ' +
'in the background-tasks view and the `/workflows` dialog (live phase ' +
'tree, token usage, cooperative pause/resume, cancel). Set ' +
'`run_in_background: true` to return a ' +
'run handle immediately in the interactive TUI and receive completion ' +
'through the conversation. Scripts run in a node:vm sandbox without ' +
'access to the filesystem or shell; all I/O happens through the ' +
'spawned agents.',
WORKFLOW_TOOL_DESCRIPTION,
Kind.Other,
WORKFLOW_PARAM_SCHEMA,
/* isOutputMarkdown */ true,
Expand Down
Loading