diff --git a/packages/core/src/tools/workflow/workflow.test.ts b/packages/core/src/tools/workflow/workflow.test.ts index e23ca2f7833..9a5923fef9c 100644 --- a/packages/core/src/tools/workflow/workflow.test.ts +++ b/packages/core/src/tools/workflow/workflow.test.ts @@ -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 { @@ -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()); + // 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/); + 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/); + // #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('')` is a blind guess and `scriptPath` wants an + // absolute path it cannot construct. + expect(description).toContain('.qwen/workflows'); + }); + + // 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/); diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index 34c73127632..73c4186f897 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -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, @@ -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 ' + @@ -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.', @@ -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 + * 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. + +\`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 \`.js\` files under \`/.qwen/workflows\` (project scope, also surfaced as \`/\` slash commands) or \`~/.qwen/workflows\` (user scope, lower precedence when both define the same name); \`workflow('')\` 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. + +**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 @@ -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,