feat(core): require an explicit user opt-in before the model launches a workflow - #9806
Conversation
… a workflow
The Workflow tool description carried judgement heuristics for orchestrating
well, but no rule about when not to orchestrate at all. Read on its own the
prose is encouragement, and a run that can dispatch up to the per-run agent
cap is a large spend to enter on inference rather than on a request.
Prepend a gate above the existing guidance: do not call the tool unless the
user asked for multi-agent orchestration, with the five forms that count as
asking under this project's own triggers -- the `workflow` keyword, the
user's own words, a skill or slash command, a named saved workflow reached
through `workflow('<name>')` or `scriptPath`, and a resume. Upstream's
`ultracode` marker is deliberately not among them: it does not exist here,
and naming it would enumerate a trigger no user can pull.
The fallback path is the load-bearing half. Without an offer-and-ask route
the model reads a refusal rule as "refuse", and a user who would have said
yes never gets asked, so the text ends by telling it to say what a workflow
would fan out over and let the user decide.
The agent cap is interpolated from `DEFAULT_MAX_AGENTS_PER_RUN` rather than
pasted, matching the rest of the description.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MDuRWMhKbqEYZXbbLhn3en
|
Thanks for the PR! Template looks good ✓ Re-run context: new commits landed since the last pass (which reviewed Problem: unchanged from the prior pass and re-verified against current main: this is a consent gap, not a crash. The Workflow description tells the model how to orchestrate well, but nothing in it says when orchestration was actually asked for — and one run can dispatch up to Direction: aligned. The file header declares the description's prose load-bearing, and this extends that pattern to the one policy the description has never carried. CHANGELOG: no direct reference to a gate like this one, but the area is clearly relevant. Size: 12 production lines (additions only) + 51 test lines, all inside Approach: scope is right, and the two commits since the last pass move it in exactly the right direction: Risk: no high-risk path matches, no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ Re-run 背景： 上一轮（审于 问题： 与上一轮判断一致，并已在当前 main 上重新核实：这是一个"授权缺口"，而不是崩溃类 bug。Workflow 工具描述告诉模型如何把编排做好，却从不说什么情况下编排是用户真正要求的——而单次运行最多可以扇出到 方向： 对齐。文件头注释声明这段描述中的文字是 load-bearing，本 PR 把同一模式扩展到描述从未承载过的那条策略。CHANGELOG 没有对这个 gate 的直接引用，但该领域明显相关。 规模： 12 行生产代码（纯新增）+ 51 行测试，全部位于 方案： 范围合理，且上一轮之后的两个提交方向完全正确： 风险： 未命中任何高风险路径，无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review Independent baseline first: for a consent gap in a tool description, the minimal correct change is a short rule at the top of the description — enumerate what counts as an explicit request, add an offer-and-ask fallback so "don't call it" can't read as "refuse", and anchor the whole thing with a test. The PR matches that baseline; this re-run reviews what changed since the last pass at Delta since the last pass — production prose untouched, tests tightened, hygiene restored:
No blockers. Test evidence (the PR's own CI — the gate never runs PR code) All checks are settled on the reviewed commit, and everything that ran is green — including
What this evidence can and cannot prove: CI settles whether the anchored prose is present, correctly placed, and correctly interpolated. The author's mutation reproductions (any→all and sentence-deletion now fail the focused suite) are their claim, and the assertions read consistently with them. CI cannot prove the model complies with the rule, and the PR says so itself. Not verified: model-side compliance — no unit suite can verify it, and the PR explicitly scopes it out. A sandboxed Real-scenario (tmux) testing: N/A — unattended CI run, and the change has no TUI surface to drive in the first place. 中文说明代码审查 先说独立基线：对于工具描述中的授权缺口，最小且正确的改动是在描述顶部加一条简短规则——列举什么算"明确请求"，加上"提议并询问"的回退路径使"不要调用"不会被读成"拒绝"，并用一个测试把整段锚定。PR 与该基线一致；本次 re-run 审的是自上一轮（ 与上一轮的差异——生产文字未动、测试收紧、卫生恢复：
无阻塞项。 测试证据（来自该 PR 自己的 CI——门禁不运行 PR 代码） 受审提交上的所有检查已落定，且运行的全部为绿——包括承载本 PR 锚定测试的 这些证据能证明与不能证明的：CI 能确认锚定的文字在场、位置正确、插值正确。作者的变异复现（any→all 与删句现在会使聚焦套件失败）是其自述，与断言内容一致。CI 无法证明模型会遵守这条规则，PR 自己也这么说。未验证：模型侧的遵从性——任何单元测试套件都无法验证，PR 明确将其划在范围之外。沙箱 真实场景（tmux）测试：N/A——无人值守 CI 运行，且该改动本来就没有可驱动的 TUI 表面。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the same clean, minimal consent gate as the last pass, now with every review finding closed and the diff back to exactly two files; the only reservation remains the unmeasurable one the PR itself names. Stepping back: the re-run asks one question — did the new commits help? They did, precisely. The production prose reviewed last time is byte-identical; The reservation, unchanged and stated plainly: nobody has observed a model complying (or not) with a gate like this, and no unit test ever will. The claim on offer is "the rule is now stated and anchored", which is exactly what the diff delivers. Post-merge, over-blocking remains the thing to watch — a user who would have said yes to a workflow never gets asked — and the anchored fallback paragraph is the sentence to tune if it shows up. The sandboxed Verdict: approve — CI is settled green on the reviewed commit, so nothing is deferred. 中文说明Confidence: 4/5 —— 与上一轮相同的干净、最小化的授权门禁；现在所有审查发现都已关闭，diff 重新收敛到恰好两个文件；唯一的保留意见仍是 PR 自己也点名的那个不可测量项。 退一步看：这次 re-run 只问一个问题——新提交有没有让事情变好？有，而且很精准。上次审到的生产文字逐字节未变； 保留意见，不变且直说：没有人观察到模型会（或不会）遵守这样的门禁，单元测试也永远无法证明。它给出的主张是"规则现在被写下并被锚定"，这正是 diff 所交付的。合并后值得留意的仍是过度阻断——本会同意使用 workflow 的用户再也没被问到——那段有锚点的回退文字就是实践出现问题时要去调整的句子。沙箱 结论：批准——受审提交上的 CI 已全部落定为绿，无需推迟。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.0)
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! ✅
Test Plan (not a blocker): src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/workflow/workflow.test.ts:165 — [probe] the new gate test anchors the gate's enumeration but not its logical load-bearers (prohibition sentences and the any-of connective) — both drift mutations stay green (probe-ver…
中文说明
无阻断问题。LGTM！✅
Test Plan（非阻断）：src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…-gate # Conflicts: # packages/core/src/tools/workflow/workflow.test.ts
The merge commit's pre-commit hook ran prettier over every file the merge staged, not just the ones this branch changes, and reformatted a type union in agent-core.ts. main's copy does not satisfy the repo's pinned prettier 3.6.1, so the hook produced a real diff in a file this PR has no business touching. Restore main's bytes to keep the PR scoped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MDuRWMhKbqEYZXbbLhn3en
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| expect(description).toMatch(/let the user decide/); | ||
| expect(description).toMatch(/skips the ask/); |
There was a problem hiding this comment.
[Suggestion] The new gate test anchors the heading, the five trigger bullets, and the offer-and-ask path, but none of its assertions cover the gate's prohibition sentences — "Do not call this tool unless the user has asked for multi-agent orchestration." and "Otherwise do not call it, however well the task would parallelize." in workflow.ts. A probe-verified mutation deletes both sentences while keeping the heading, bullets, and offer paragraph, and every assertion stays green: the description loses the negative instruction that is the point of the gate, silently reintroducing the unrequested multi-agent dispatch this PR was written to prevent. Adding the two anchors below kills the mutant and stays green on the real prose.
Witness (probe in a scratch tree at the reviewed commit):
MUTANT (both prohibition sentences removed): Tests 53 passed (53)
MUTANT + proposed anchors: AssertionError: expected 'Execute a workflow script that orches…' to match /Do not call this tool unless/
ORIGINAL + proposed anchors: Tests 53 passed (53)
| expect(description).toMatch(/let the user decide/); | |
| expect(description).toMatch(/skips the ask/); | |
| expect(description).toMatch(/let the user decide/); | |
| expect(description).toMatch(/skips the ask/); | |
| expect(description).toMatch(/Do not call this tool unless/); | |
| expect(description).toMatch(/Otherwise do not call it/); |
中文说明
新增的门禁测试锚定了标题、五种触发形式与“提议并询问”路径，但没有任何断言覆盖门禁中的禁止句——即 workflow.ts 中的 “Do not call this tool unless the user has asked for multi-agent orchestration.” 与 “Otherwise do not call it, however well the task would parallelize.”。经探针验证的变异（删除这两句、保留标题、触发列表与提议段落）下所有断言仍然全绿：描述由此失去门禁真正依赖的否定指令，悄悄重新引入本 PR 要防止的未经请求的多智能体扇出。补上下面两个锚点后该变异被杀死，且在真实文字上保持绿色。
见证（受审提交上的隔离树探针）：MUTANT（删除两句禁止句）：53 通过；MUTANT + 建议锚点：AssertionError（无法匹配 /Do not call this tool unless/）；原文 + 建议锚点：53 通过。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Fixed in c25fca4. The existing gate test now pins both prohibition sentences exactly. Independent mutation reproduction confirmed that deleting them previously left the focused suite green and now fails. Verification: workflow.test.ts 53/53; npm run build; npm run typecheck; targeted ESLint and Prettier; pre-commit hook.
| expect(description).toMatch(/contains the word `workflow`/); | ||
| expect(description).toMatch(/in their own words/); |
There was a problem hiding this comment.
[Suggestion] The test anchors each of the five trigger bullets but not the disjunction that makes them sufficient conditions — "It counts as requested when any of these holds:" (workflow.ts:814) has no assertion anywhere; the phrase appears only in the description. A probe-verified mutation flips the quantifier to "only when all of these hold:" and the whole suite stays green. All-five-at-once is effectively unsatisfiable (one message would need the keyword, a paraphrase, a skill/slash command, a named saved workflow, and a resume id together), so the gate would then refuse explicit orchestration requests — over-blocking, which this test's own comment names as the change's one real failure mode, shipped by a green run of the test written to catch exactly that. The anchor below kills the mutant and passes on the real prose.
Witness (probe in a scratch tree at the reviewed commit):
MUTANT ("It counts as requested only when all of these hold:"): Tests 1 passed (1)
MUTANT + proposed anchor: AssertionError: expected 'Execute a workflow script that orches…' to match /counts as requested when any of these…/
ORIGINAL + proposed anchor: green
| expect(description).toMatch(/contains the word `workflow`/); | |
| expect(description).toMatch(/in their own words/); | |
| expect(description).toMatch(/contains the word `workflow`/); | |
| expect(description).toMatch(/in their own words/); | |
| expect(description).toMatch(/counts as requested when any of these holds/); |
中文说明
测试锚定了五种触发形式中的每一种，但没有锚定使它们成为充分条件的析取句——“It counts as requested when any of these holds:”（workflow.ts:814）没有任何断言覆盖，该短语只出现在描述中。经探针验证的变异把量词翻转为 “only when all of these hold:” 后整个套件仍然全绿。五种形式同时满足实际上不可能（一条消息需要同时包含关键词、同义转述、skill/斜杠命令、指名的已保存 workflow 和 resume id），门禁因此会拒绝明确的编排请求——这正是本测试自己的注释点名的唯一真实失败模式（过度阻断），且会在这条本应捕获它的测试全绿的情况下被放行。补上下面这个锚点后该变异被杀死，在真实文字上保持绿色。
见证（受审提交上的隔离树探针）：MUTANT（翻转为 all）：1 通过；MUTANT + 建议锚点：AssertionError（无法匹配 /counts as requested when any of these…/）；原文 + 建议锚点：绿色。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Fixed in c25fca4. The gate test now pins the any-of connective exactly. Independent mutation reproduction confirmed that changing any to all previously left the focused suite green and now fails. Verification: workflow.test.ts 53/53; npm run build; npm run typecheck; targeted ESLint and Prettier; pre-commit hook.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/config/config.workflows.test.ts — no such file or directory; src/config/config.workflow-registration.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 104 passed · 0 failed · 104 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：104 通过 · 0 失败 · 104 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9806 Deep Verification —
|
| cell | build | oracle | result |
|---|---|---|---|
| head | merge-ref HEAD (packages/core built at HEAD by CI) |
gate index in rendered description | 74 (present, first heading) |
| base | HEAD^1 worktree, packages/core rebuilt there |
same | −1 (absent) |
| both | pitch index | **What a workflow is for** position |
head 1223 / base 74 |
| both | byte equivalence | head − gate block ≡ base |
exact (Δ = 1149 B = the gate block only) |
Witness: 01-ab-gate-flip-base-vs-head.png — 18/18 head assertions (gate present, first heading, precedes pitch, five forms enumerated, no ultracode, prohibition + escape-hatch sentences, cap interpolated as 1000, wire form identical) and 5/5 base assertions (expected absence encoded as pass).
Control purity: the emitted workflow.js files from the two builds differ by exactly the 12 added gate lines (logs/emitted-workflow-js.diff) — the base-side tsc run reported unrelated type errors in nested-tree modules (telemetry/ajv type resolution artifacts), but the module under test emitted byte-identically to head-minus-hunk, which is the strongest possible evidence the control differs by nothing else. packages/core has zero workspace dependencies and the PR leaves package.json/package-lock.json untouched, so the shared root node_modules is a clean control; the core-local node_modules (ajv v8 etc.) was symlinked into the base tree and readlink -f confirms it resolves into the head tree's identical dependency set — no @qwen-code/* links are crossed because core has none.
Corrections
- PR body, Reviewer Test Plan: "Observed:
Test Files 3 passed (3),Tests 53 passed (53)" — re-running the exact same command gives 3 files passed but 63 tests (workflow.test.ts53 +config.workflows.test.ts4 +config.workflow-registration.test.ts6). The 53 figure is the single-file count. The gate itself is green; this is a description correction, not a code issue.
Findings (none blocking)
- Low — the reminder clause is true only on the interactive TUI surface. The gate's first bullet says the
workflowkeyword is confirmed by "a system reminder". The mechanism exists and is wired —AppContainer.tsx:2655-2678detects the keyword on the user's original text and prepends<system-reminder>…</system-reminder>to the submitted turn, gated onisWorkflowsEnabled()and thedisableWorkflowKeywordTriggersetting, with telemetry — but that is the only production call site of bothdetectWorkflowKeywordandbuildWorkflowSteeringNotice(scripted census: 1 each, both inAppContainer.tsx). Headless (nonInteractiveCli.ts), ACP sessions, and channel surfaces never inject it, so on those surfaces the clause overstates. Practical impact nil — the keyword form of the gate is still satisfied model-side because the model reads the user message — and the wiring is pre-existing, so this is a prose-precision note: "in the interactive TUI, a system reminder confirms it" would be exact. - Info — the prepend wiring has no direct test. Detector semantics (
workflow-keyword.test.ts) and the footer indicator (Footer.test.tsx:328) are tested; thehandleSubmitprepend itself is not pinned by any test. Pre-existing gap, now referenced by model-visible prose. - Info — expected mutation survivor. Pasting
1000instead of interpolating${DEFAULT_MAX_AGENTS_PER_RUN}survives the new test (content-equivalent while the constant is 1000). The pin becomes live the moment the constant moves. Completeness note, not a defect.
Not covered
- Model-behavior shift: whether the gate actually reduces unrequested fan-outs is not measurable by unit tests; the PR honestly says so. Not attempted.
- Per-commit attribution: metadata lists 7 commits; the checkout is depth-2 (merge, base tip, PR head only), so only the aggregate
HEAD^1..HEADdiff was exercised. The aggregate diff is exactly the two expected files (+63 lines). - Repo-wide suites: only the three suites named in the PR's test plan were run (63/63), per targeted-gate policy.
- Live TUI E2E of the reminder flow: wiring verified by census plus its existing unit tests, not by driving a real session.
- Snapshot
baseRefOiddrift: the metadata snapshot names base5cff52c6…(fix(goal): count catalog previews in the unit their budget is written in #9835), newer than the merge-ref base717ad101e2. Verified directly:git diff HEAD^1 5cff52c6touches 37 files but zero underpackages/core/src/tools/workflow/, so the PR's surface merges conflict-free against the newer main.
Methodology
Environment: CI container (node:22-bookworm, Node v22.23.2), working tree at merge-ref 254b915716 with npm ci + npm run build already run at HEAD. Base control: git worktree add tmp/base-tree HEAD^1, packages/core rebuilt there via node scripts/build_package.js (base tree removed after capture). Harnesses (harness-ab.mjs, harness-claims.mjs, run-mutations.py, print-matrix.mjs) import the compiled dist/ directly by absolute path — real constructors, no stubs; every assertion is a scripted comparison that can fail, and expected base-arm absences are encoded as passes. Mutation matrix: each mutant edits workflow.ts in place, runs npx vitest run src/tools/workflow/workflow.test.ts, records the summary and failing assertion text, then restores via git checkout -- (tree verified clean afterwards). Raw logs: logs/ab-run.log, logs/claims-run.log, logs/gate-baseline.log, logs/mutant-M*.log, logs/base-build.log, logs/emitted-workflow-js.diff. Self-audit note: an early call-site census using grep --include='*.ts' missed the .tsx wiring file and briefly suggested the keyword trigger was unwired; the language-agnostic census harness corrected this, and the corrected state (wired, one call site) is what the report carries.
Flakiness gate log
rounds=5 files=1 skipped=0
file packages/core/src/tools/workflow/workflow.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/workflow/workflow.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/tools/workflow/workflow.test.ts: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/tools/workflow/workflow.test.ts: P (exit 0)
round 2 · packages/core/src/tools/workflow/workflow.test.ts: P (exit 0)
round 3 · packages/core/src/tools/workflow/workflow.test.ts: P (exit 0)
round 4 · packages/core/src/tools/workflow/workflow.test.ts: P (exit 0)
round 5 · packages/core/src/tools/workflow/workflow.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Local verification report for PR #9806Verified the workflow opt-in gate change in a clean local worktree checked out from Environment
What was verified
ResultsBuild: ✅ success ScreenshotsTerminal output from the test run: Standalone gate verification: ConclusionThe PR applies cleanly, builds cleanly, type-checks cleanly, and the new opt-in gate is present in the expected location with the expected content. From a local build/test perspective this looks ready to merge. 🌐 中文版（点击展开）PR #9806 本地验证报告在从 环境
验证内容
结果构建： ✅ 成功 截图测试运行的终端输出： 独立的门槛验证： 结论该 PR 应用干净、构建干净、类型检查干净，且新的显式请求门槛出现在预期位置并包含预期内容。从本地构建/测试的角度来看，已可合并。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…QwenLM#9827) Merging origin/main brought ToolNames.REQUEST_SHUTDOWN (QwenLM#9806) but no TOOL_NAME_ALIASES entry, which the resolveToolName exhaustiveness test added on this branch pins. Map request_shutdown / RequestShutdown so permission rules can address the tool.
…he model (QwenLM#9829) * fix(core): make permissions.allow restrict the tool schemas sent to the model (QwenLM#9827) permissions.allow only auto-approved calls; it never gated tool registration, so the outgoing tools array kept every built-in schema even when an allowlist was configured — contradicting the settings docs migration table ("unlisted tools are disabled at registry level") and breaking backends like llama.cpp that compile all tool schemas into a single grammar. - Activate a registry-level allowlist when settings.permissions.allow has at least one valid rule: built-in tools not covered by any allow rule are no longer registered (absent from /tools and the API request). MCP tools and the structured_output contract stay exempt; session-granted rules ("always allow", skill allowedTools) extend membership but never activate the allowlist mid-session. --allowed-tools / SDK allowedTools / legacy tools.allowed keep their pure auto-approval semantics. - Complete the rule alias map so the display names shown by /tools (SendMessage, UpdateGoal, ...) match in allow/deny rules. * fix(core): honor permissions.allow in the list_directory opt-in gate (QwenLM#9827) isLsToolEnabled() only read tools.listDirectory.enabled and the coreTools allowlist, so an explicitly allowlisted list_directory passed PermissionManager.isToolEnabled() but was never registered — absent from /tools and the model request, with calls failing TOOL_NOT_REGISTERED. This broke the documented tools.core -> permissions.allow migration equivalence for exactly this tool. Consult getRegistryAllowList() with the same coverage semantics the registry gate uses (toolMatchesRuleToolName, so Read / ListFiles / specifier forms all count). * fix(core): keep plan-mode lifecycle tools registered under the allowlist (QwenLM#9827) The permissions.allow registry gate covered exit_plan_mode / enter_plan_mode / ask_user_question, so the exact reporter configuration unregistered them. The plan-mode system reminder still instructs the model to present its plan by calling exit_plan_mode, whose schema is then never sent, so the sanctioned plan flow cannot complete. Exempt the three plan-mode lifecycle tools alongside structured_output (same synthetic- system-tool class the CORE_TOOLS docstring names; deny rules still apply). * docs(sdk): correct allowedTools registry-allowlist contract (QwenLM#9827) The JSDoc added for QueryOptions.allowedTools (and the coreTools block) claimed the SDK allowedTools param activates the registry allowlist and hides unlisted built-in schemas. It does not: ProcessTransport maps it to the CLI --allowed-tools flag, and this PR's CLI wiring builds registryAllowList only from settings.permissions.allow. Reword both JSDoc blocks and the two hand-maintained SDK doc pages (sdk-typescript.md, sdk-typescript/README.md) to the shipped contract: allowedTools stays a pure auto-approval grant; only permissions.allow in settings.json (requires restart) activates the registry allowlist. * docs(settings): note plan-mode lifecycle exemption in the allowlist (QwenLM#9827) The permissions.allow registry-allowlist exemption list named only MCP tools and the structured_output contract. Add the plan-mode lifecycle tools (exit_plan_mode / enter_plan_mode / ask_user_question) exempted in b8ba258 so the documented exemption set matches the gate. * fix(core): exempt the computer_use__* family from the registry allowlist (QwenLM#9827) * fix(core): gate command-discovered tools through the registry allowlist (QwenLM#9827) * fix(core): make registry-allowlist membership monotonic within the session (QwenLM#9827) * fix(core): narrow the skill allowedTools grant contract to restart-scoped registration (QwenLM#9827) * fix(core): count ask rules toward registry-allowlist membership (QwenLM#9827) A tool covered only by a permissions.ask rule was silently deregistered whenever the permissions.allow registry allowlist was active: allow ["ReadFile"] + ask ["Shell"] hid the whole shell family from the model, so the documented "always require user confirmation" silently became "tool unavailable" and the ask rule could never fire. Ask rules express "this tool must stay usable, with confirmation", so they now count toward registry membership (frozen at startup for the same restart-scoped monotonicity as allow rules). * docs(settings): note that ask rules keep tools registered under allowlist (QwenLM#9827) * test(cli): pin registry-allowlist strip in bare mode (QwenLM#9827) The wiring tests only covered the safe-mode half of registryAllowList: bareMode || safeMode ? undefined : ... — a mutant dropping the bareMode guard survived the suite and would activate the allowlist from settings while bare mode strips those same rules from the merged allow set, leaving the bare registry's minimal toolset ungated. Mirror the safe-mode test for --bare. * fix(core): attribute registry-allowlist misses to permissions.allow (QwenLM#9827) An allowlist-miss rejection surfaced as "Qwen Code requires permission to use X, but that permission was declined" citing a deny rule that does not exist (findMatchingDenyRule finds nothing) and never mentioning permissions.allow. When no deny rule matched and the registry allowlist is active, emit a distinct message pointing at the real config knob. * test(core): pin resolveToolName coverage of every ToolNames entry (QwenLM#9827) TOOL_NAME_ALIASES hand-maintains the canonical/display-name mappings that tool-names.ts declares; nothing enforced the sync, so a tool added to tool-names.ts without an alias entry would compile, pass every test, and silently never match a permission rule — the exact QwenLM#9827 bug class, now with higher stakes since a missed entry also breaks allowlist coverage. Walk every ToolNames/ToolDisplayNames pair and assert it round-trips through resolveToolName. * fix(core): expose isPermissionsAllowListActive on scoped PM shims (QwenLM#9827) * fix(core): honour ask-only list_directory coverage in the opt-in gate (QwenLM#9827) * docs: align registry-allowlist contract wording across docs and JSDoc (QwenLM#9827) * docs: scope settings.md removal and whole-tool-deny claims precisely (QwenLM#9827) * fix(core): count merged allow coverage in the list_directory opt-in gate (QwenLM#9827) isLsToolEnabled() scanned only the settings-sourced getRegistryAllowList() for allow coverage while PermissionManager.isToolEnabled() counts the merged allow set (settings + --allowed-tools + SDK allowedTools + legacy tools.allowed). Under an active allowlist, list_directory covered only by a merged rule passed isToolEnabled but was never offered to registerLazy — it vanished from /tools and the model request while calls failed TOOL_NOT_REGISTERED. Count the merged allow set for coverage (activation still requires a valid settings rule) and filter empty/whitespace-only entries from activation exactly like PermissionManager.initialize's parseRules does, so a degenerate [""] entry cannot activate the gate here while the permission system reports the allowlist inactive. * test(core): pin activation source and merged-allow coverage of the list_directory gate (QwenLM#9827) Every existing isLsToolEnabled test fed the identical array as both allow and registryAllowList, so the settings-only vs merged-allow distinction was unpinned and the R4-1 divergence shipped uncovered. Add three cases shaped like the CLI wiring: coverage by a merged (non-settings) allow rule under an active allowlist registers the tool; merged-only coverage with no settings rule does not activate the allowlist; an empty settings entry ([""]) does not activate it either. * fix(core): attribute scheduler denials to permissions.allow only for uncovered tools (QwenLM#9827) The allowlist-miss message fired for any disabled tool with no matching deny rule while the allowlist is active — including tools rejected by the legacy coreTools gate that ARE covered by an allow rule, where 'not covered by any permissions.allow rule' is wrong and the remediation a no-op. Expose isCoveredByAllowOrAskRule on PermissionManager and take the allowlist branch only when the tool is genuinely uncovered; covered tools fall back to the generic declined message. The optional call keeps scoped PermissionManager shims (installed via 'as unknown as PermissionManager') from throwing until they grow the delegation. * test(core): pin the covered-tool fallback for scheduler denial messages (QwenLM#9827) Add a scheduler-level case where the allowlist is active, no deny rule matches, and the disabled tool IS covered by an allow rule (the legacy coreTools gate shape): the message must be the generic declined one, not the permissions.allow attribution. Also make the existing allowlist-miss stub explicit about coverage. * fix(core): register request_shutdown in the permission rule alias map (QwenLM#9827) Merging origin/main brought ToolNames.REQUEST_SHUTDOWN (QwenLM#9806) but no TOOL_NAME_ALIASES entry, which the resolveToolName exhaustiveness test added on this branch pins. Map request_shutdown / RequestShutdown so permission rules can address the tool. * fix(core): guard the list_directory allowlist gate against non-string rules (QwenLM#9827) isLsToolEnabled()'s activation check and coverage scan called raw.trim() / parseRule(raw) directly while PermissionManager.initialize computes the same thing through parseRules, whose r && r.trim() filter skips falsy entries. Settings load performs no element-type validation (the schema declares only type: array), so a stray null in settings.permissions.allow/ask — or in the legacy tools.allowed key riding the merged coverage set — threw TypeError during createToolRegistry and crashed startup while PermissionManager.initialize tolerated the same settings file. Mirror the parseRules guard with a typeof check in both the activation check and the coverage predicate, and pin both arms (tolerated entries still activate/cover, a [null]-only list keeps the gate closed). * fix(core): exempt task_stop from the permissions.allow registry gate (QwenLM#9827) task_stop satisfies the PR's own two written exemption criteria but was missing from the set: it is shouldDefer=true (task-stop.ts), the exact deferred-schema property the computer_use__* exemption cites, and it is advertised to the model by a registered tool's copy — run_shell_command's schema says to use task_stop to stop a background command (and not to use broad process-name kills), and the background-promotion result instructs task_stop({ task_id }) verbatim. Under the reporter configuration the suite pins, run_shell_command stays listed while task_stop was gated out, so the sanctioned stop flow failed. Add the exemption and pin it next to the plan-mode exemption tests, including that a whole-tool deny rule still wins via the existing evaluate pass. * fix(core): keep shim denials on the pre-QwenLM#9827 message when coverage is unknown (QwenLM#9827) The optional isCoveredByAllowOrAskRule call's : true fallback routed shim-mediated rejections of COVERED tools into the new allowlist-attribution message, contradicting the comment above it ('they keep the pre-QwenLM#9827 message meanwhile'). Both production shims (memory-scoped-agent-config.ts, skillReviewAgentPlanner.ts) Pick a partial interface without isCoveredByAllowOrAskRule, so for them the ternary always took the allowlist arm — telling the user a covered tool 'is not covered by any permissions.allow rule' when a different gate (e.g. the legacy coreTools allowlist) rejected it. Flip the fallback to false so unknown coverage stays on the pre-QwenLM#9827 declined message, and update the shim test to pin that message instead of the allowlist one. * test(core): pin that ask-only rules never activate the allowlist (QwenLM#9827) The suite pins ask rules counting toward allowlist membership, but nothing pins the complementary activation boundary: no test constructed a PermissionManager with only permissionsAsk (no permissionsAllow) and asserted the allowlist stays inactive. Current behavior is correct; this guards against a future edit folding ask rules into activation, which would turn an ask-only posture (permissions.ask: ["Shell"], no allow rules — a natural 'always confirm shell' config) into an active allowlist that deregisters every unlisted built-in. The nearest existing test ('no allow rules → allowlist inactive') uses no rules at all and would still pass. * fix(core): exempt tool_search from the permissions.allow registry gate (QwenLM#9827) Under a narrow active allowlist, tool_search itself was gated out of the registry. Without ToolSearch, client.ts resolveDeferredToolsForReminder eagerly force-reveals every registered deferred tool (all mcp__* and the deferred computer_use__* family) into the eager model request, and preloadDeferredToolsWithinBudget early-returns — inverting the schema-shrink goal into maximal schema bloat for exactly the deferred families the other exemptions preserve for ToolSearch discoverability. Pre-QwenLM#9827 tool_search always bypassed the legacy coreTools gate as a non-core tool. * test(core): pin the deny-rule arm's precedence in the scheduler permission message (QwenLM#9827) The three-way message branch in CoreToolScheduler covers the allowlist-miss arm and the generic fallback arm, but every findMatchingDenyRule mock returned undefined, so the deny-rule arm — whose position FIRST in the if/else-if chain is what makes a real denial cite the matching rule instead of the allowlist attribution — had no scheduler-level coverage. Add two tests where findMatchingDenyRule returns a matching rule: one with the allowlist arm armed (active allowlist + uncovered tool) pinning the if/else-if ordering, one without an active allowlist pinning the deny arm over the generic declined fallback. Mutation-checked: disabling the deny arm fails both tests. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): pin deny/ask sibling semantics at the discovery gate (QwenLM#9827) The discovery-gate test built its PermissionManager with EMPTY ask/deny lists, so the gate's two documented sibling semantics were unpinned: settings.md says a whole-tool deny rule removes a discovered tool from the registry even under an active allowlist, and an ask rule keeps a discovered tool registered ("always require confirmation" must never silently become "tool unavailable"). Add two discovery-gate tests with deny-covered and ask-covered PermissionManager configurations: the denied tool is also allow-covered so only the deny branch of isToolEnabled can reject it, and the ask test carries an uncovered control tool proving the gate is active in the same run. Mutation-checked: ignoring deny decisions fails the deny test only; dropping ask coverage from isCoveredByAllowOrAskRule fails the ask test only. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs: match the allowlist activation wording to the real predicate (QwenLM#9827) Four surfaces said the permissions.allow registry allowlist activates "when at least one allow rule is configured", but PermissionManager.initialize computes activation as at least one VALID rule from settings.permissions.allow only (getRegistryAllowList): a malformed entry never activates it, and auto-approval-only sources such as the --allowed-tools CLI flag / the SDK allowedTools parameter never do either. Reword settings.md, the SDK docs, the sdk-typescript README and the coreTools JSDoc to the exact predicate, and complete their exemption lists with task_stop and tool_search, which isToolEnabled exempts but the docs did not name. Docs-only. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Released in v0.22.2. |
3 participants
What this PR does
Adds a rule to the Workflow tool description telling the model not to start a workflow unless the user actually asked for multi-agent orchestration, and enumerates the five ways a user can ask under this project's own triggers: the
workflowkeyword, a request in the user's own words, a skill or slash command whose instructions say to use the tool, naming a saved workflow to run, and asking to resume an earlier run.The rule is placed above the existing "What a workflow is for" guidance rather than below it, so that it frames that guidance instead of reading as a footnote to it. It closes with an offer-and-ask path: when a workflow really would be the better tool but nobody asked for one, the model should say in a sentence what it would fan out over and roughly how many agents that is, then let the user decide.
Two deliberate choices. Upstream's opt-in marker is the keyword
ultracode, which does not exist in this repository — the string appears once in the whole tree, inside a comment — so it is not among the enumerated forms; naming it would advertise a trigger no user here can pull. And the agent count in the rule's justification is interpolated fromDEFAULT_MAX_AGENTS_PER_RUNrather than pasted as prose, matching how every other cap in this description is written, so raising the cap moves this sentence too.Why it's needed
The description already carries a policy layer about orchestrating well — prefer
pipeline()over a barrier, verify a finding before reporting it, log what was dropped. What it has never carried is a rule about when not to orchestrate at all. Read on its own, that prose is encouragement, and the model reasonably concludes that a task which would parallelize nicely is a task that should be parallelized.That is an expensive default for this particular tool. A single run can dispatch up to the per-run agent cap and spend tokens accordingly, and today the only thing standing between an inferred fan-out and the spend is an approval dialog that says
Run a workflow script (4127 chars)and nothing else. Until that dialog says more, the cheapest place to require an explicit request is the description the model reads before it decides.This is a narrowing change: its intended effect is that the tool gets called less often, and only when someone asked for it.
Reviewer Test Plan
How to verify
The change is prose inside
WORKFLOW_TOOL_DESCRIPTIONplus one test that anchors it. Run the tool's own suite and the two adjacent config suites frompackages/core:Observed:
Test Files 3 passed (3),Tests 53 passed (53).The new test,
description gates the tool on an explicit user request, asserts more than presence. It checks that the gate's index in the description precedes the index of the**What a workflow is for**heading, because position is the point — a rule placed after the pitch qualifies the pitch instead of framing it. It checks that each of the five enumerated forms is a real trigger in this codebase. It asserts the description does not containultracode. It anchors the offer-and-ask sentences, which are the guard against the change's one real failure mode. And it anchors the agent count through the exported constant rather than as a literal.To confirm the test is anchored to the prose rather than passing vacuously, revert only the production file and re-run:
git stash push -- packages/core/src/tools/workflow/workflow.ts && cd packages/core && npx vitest run src/tools/workflow/workflow.test.ts. Observed: the new test fails, every other test in the file still passes.WORKFLOW_TOOL_DESCRIPTIONhas exactly one consumer — theWorkflowToolconstructor in the same file — and no snapshot captures it, so no other suite is affected.Evidence (Before & After)
N/A — the change is model-facing prose in a tool description, with no TUI surface.
Tested on
Environment
Unit tests only, via
npx vitest runfrompackages/core. Also checked withnpx tsc --noEmit -p tsconfig.json(exit 0),npx prettier --check, andnpx eslinton both changed files.Risk & Scope
scriptparameter's authoring contract changes.Linked Issues
None.
中文说明
这个 PR 做了什么
在 Workflow 工具描述中加入一条规则:除非用户确实要求了多智能体编排,否则模型不应启动 workflow;并列出在本项目自身的触发方式下,用户"提出要求"的五种形式:
workflow关键词、用户用自己的话提出、指示使用该工具的 skill 或斜杠命令、指名运行某个已保存的 workflow、以及要求恢复此前的运行。这条规则被放在现有的 "What a workflow is for" 指引之上而非之下,这样它是在为那段指引定框架,而不是读起来像它的一条脚注。规则结尾给出一条"提议并询问"的路径:当 workflow 确实是更合适的工具但没有人提出要求时,模型应当用一句话说明它会在什么上做扇出、大致需要多少个 agent,然后把决定权交还给用户。
有两处是刻意的取舍。上游用于此目的的标记是关键词
ultracode,而它在本仓库中并不存在——这个字符串在整棵代码树中只出现过一次,还是在一条注释里——因此它没有被列入这五种形式;列出它等于向模型宣传一个本地用户根本无法触发的入口。另外,规则中用于论证的 agent 数量是从DEFAULT_MAX_AGENTS_PER_RUN插值而来,而不是作为文字硬写,这与该描述中其它所有上限的写法一致,因此上调该上限时这句话会同步变化。为什么需要
该描述中已经有一层关于"如何编排得好"的策略文字——优先用
pipeline()而非 barrier、报告结论前先验证、把被丢弃的部分log()出来。它从来没有承载的,是一条关于"何时根本不该编排"的规则。单独去读,那些文字就是鼓励,模型于是合理地得出结论:一个适合并行化的任务就是一个应该被并行化的任务。对这个工具而言,这是一个代价高昂的默认行为。单次运行最多可以派发到每次运行的 agent 上限,并相应地消耗 token,而目前挡在"推测出来的扇出"与这笔开销之间的,只有一个只会显示
Run a workflow script (4127 chars)的审批对话框。在那个对话框能说出更多信息之前,要求用户明确提出请求,成本最低的位置就是模型做决定前所读的这段描述。这是一个收紧性的改动:它的预期效果是这个工具被调用得更少,并且只在有人提出要求时才被调用。
审阅者测试计划
如何验证
改动是
WORKFLOW_TOOL_DESCRIPTION内部的文字,外加一个为其兜底的测试。在packages/core下运行该工具自身的测试与两个相邻的 config 测试:实测结果:
Test Files 3 passed (3)、Tests 53 passed (53)。新增的测试
description gates the tool on an explicit user request断言的不只是"存在"。它检查规则在描述中的位置早于**What a workflow is for**标题的位置,因为位置本身就是要点——放在推介之后的规则是在给推介加限定,而不是在给它定框架。它检查列出的五种形式每一种都是本代码库中真实存在的触发方式。它断言描述中不包含ultracode。它为"提议并询问"的那两句话加了锚点,那是本次改动唯一真实失败模式的防线。它还通过导出常量而非字面量来锚定 agent 数量。要确认这个测试是真的锚定在这段文字上、而不是空转通过,可以只回滚生产文件后重跑:
git stash push -- packages/core/src/tools/workflow/workflow.ts && cd packages/core && npx vitest run src/tools/workflow/workflow.test.ts。实测结果:新测试失败,该文件中其它所有测试仍然通过。WORKFLOW_TOOL_DESCRIPTION只有一个消费者——同一文件中的WorkflowTool构造函数——并且没有任何 snapshot 捕获它,因此不会影响其它测试套件。证据(前后对比)
N/A——本次改动是工具描述中面向模型的文字,没有 TUI 表面。
测试环境
仅单元测试,通过在
packages/core下执行npx vitest run。另外对两个改动文件执行了npx tsc --noEmit -p tsconfig.json(退出码 0)、npx prettier --check与npx eslint。风险与范围
script参数的编写契约,均未改变。关联 Issue
无。