Skip to content

feat(workflows): share a per-turn token budget from a +Nk directive - #11917

Merged
qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:feat/workflow-turn-budget
Sep 15, 2026
Merged

qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:feat/workflow-turn-budget

Conversation

@qqqys

@qqqys qqqys commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

A user can now size a whole turn by writing a token target in the message — +500k, +1m, +2.5m, or "use 300k tokens". Inside a workflow script, budget.total becomes that target and budget.spent() counts every output token the session has been charged since the turn began: the main loop and every agent of every workflow, not only the run that is asking. budget.remaining() gates agent() the same way the existing cap does, so a script can loop until the turn's budget is spent, or scale a fan-out to it before dispatching anything.

The directive is read from what the user typed. System reminders prepended to the message, @-referenced file and MCP resource content (which ACP may place before the prompt), and code are stripped before parsing, and a slash command's arguments never count. Only a user query or its retry carries a directive; cron, goal, notification and teammate turns start with none. The turn opens in the one place every front end's turns pass through, so the TUI, ACP, the daemon and Web Shell behave the same. A side question asked while a turn is running does not move the turn, a retry of the same prompt keeps its starting point, and a workflow reads the turn once at launch, so a run that outlives its turn keeps measuring against the turn it started in.

The per-run operator cap is unchanged: with no directive, QWEN_CODE_MAX_TOKENS_PER_WORKFLOW still caps each run and spent() still counts that run's agents. What a run shows about itself stays per run everywhere it did before — /workflows, the background tasks view, snapshots and the completion notification report the run's own spend, and a turn target is not recorded as that run's cap. The tool result adds the turn's standing beside the run's own figure (tokens: 12000 spent by this run · 150000 / 500000 this turn (+500k directive)), the usage banner and approval dialog name the target that will apply, and the tool description and the workflow-authoring skill describe the turn semantics, with a loop-until-budget and a static-scaling pattern.

Two error messages change. A refused dispatch now reads Workflow <runId>: token budget exceeded (N / M output tokens). Stopping further agent() calls., and hitting the agent cap now explains the usual cause — a loop on budget.remaining() with no target set, where remaining() is Infinity — and the two ways out.

Why it's needed

budget was an env-only, per-run cap. A user could not say in a message how much a turn may cost, and a script could only size its work to a guess, because the one number it could read measured its own run while the turn's real spend — the main loop, other runs, retries — went uncounted. The authoring reference already taught while (budget.total && budget.remaining() > 50_000), but in practice budget.total was almost always null, so that loop either never ran or, without the guard, ran to the 1000-agent cap. This follows Claude Code, where budget.total is the turn's +500k target and spent() is the session's output-token delta since the turn started.

Reviewer Test Plan

How to verify

Start a session and send a message ending in +500k that asks for a small workflow whose script returns [budget.total, budget.spent(), budget.remaining()]. Expect total to be 500000 and spent() to already include the tokens the main loop spent answering this turn before the workflow launched; the tool result's run block should show the run's own spend and the turn's spent / 500000 separately. In the same turn, a script that dispatches agents in a loop guarded by budget.remaining() > 50_000 should stop on its own before the target.

Send the same request without a directive and confirm nothing changed: budget.total is null, and the run block reads tokens: N spent (no cap). With QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=1000 set and no directive, the run block still reads tokens: N / 1000 spent and the cap applies per run as before; with both set, the directive wins.

Open /workflows for the directive run and confirm it shows the run's own token spend with no cap, not the turn's numbers. Put +900k only inside an @-referenced file or inside backticks, and confirm it sets no target.

Unit tests for the touched areas:

cd packages/core && npx vitest run src/core/turn-budget.test.ts src/telemetry/uiTelemetry.test.ts src/agents/runtime/workflow-budget.test.ts src/agents/runtime/workflow-budget-ledger.test.ts src/agents/runtime/workflow-orchestrator.test.ts src/agents/runtime/workflow-runner.test.ts src/agents/runtime/workflow-sandbox.test.ts src/agents/workflow-run-registry.test.ts src/tools/workflow/workflow.test.ts src/skills/bundled/workflow-authoring/SKILL.test.ts src/core/client.test.ts

11 test files, 1333 tests pass locally (Linux, Node 22). ESLint and Prettier are clean on the changed files, and the core package type-checks.

The ledger test drives the real LoggingContentGenerator, with nothing on the telemetry path mocked, and shows a main-loop call and a subagent call both landing in the total a turn budget is measured against.

Evidence (Before & After)

N/A — no TUI layout change. The affected surfaces are the script's budget global, the model-visible run block and error messages, and the one-time usage banner text.

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

Unit tests only.

Risk & Scope

  • Main risk or tradeoff: with no directive and no env cap, budget.spent() now reports the turn's spend instead of the run's, so a saved workflow that logged budget.spent() as "what this run cost" reads a larger number. budget.total stays null and nothing is gated in that case, and the run's own spend is still reported everywhere the harness shows it. The env cap keeps its per-run meaning exactly.
  • Main risk or tradeoff: the size ceiling on the inline fallback tool description (used only when the model cannot load the workflow-authoring skill) moves from 24,000 to 25,000 characters. On main that description already stood at 23,973, so no description of the turn budget fits under the old figure; the skill grows by about 740 characters and the fallback now stands near 24,700.
  • Main risk or tradeoff: a turn target is a soft ceiling like the existing cap — agents already in flight keep spending, and the main loop spends alongside, so a turn can end somewhat above its target. The skill says so.
  • Not validated / out of scope: the directive text stays in the message the model sees; it is not stripped. A directive does not persist past its turn. There is no setting or UI for a budget beyond the message. Registry, snapshot and /workflows do not gain turn-level counters.
  • Breaking changes / migration notes: none at the API level. The two error messages above change wording; the budget error keeps the run id and both numbers.

Linked Issues

Part of #11013

中文说明

这个 PR 做了什么

用户现在可以在消息里给整轮设定 token 目标——+500k+1m+2.5m,或者"use 300k tokens"。workflow 脚本里的 budget.total 就是这个目标,budget.spent() 统计本会话从这一轮开始以来的全部输出 token:主循环和所有 workflow 的所有 agent,而不只是发问的这一次运行。budget.remaining() 像现有上限一样拦截 agent(),所以脚本可以一直循环到本轮预算用完,也可以在派发之前就按预算决定扇出规模。

指令只从用户自己输入的文字里读取。解析前会去掉消息前面的系统提醒、@ 引用的文件和 MCP 资源内容(ACP 可能把它们放在提示词之前)以及代码,slash 命令的参数永远不算。只有用户提问及其重试会携带指令;cron、goal、通知和 teammate 开启的轮次不带指令。轮次在所有前端都要经过的同一处开启,因此 TUI、ACP、daemon 和 Web Shell 行为一致。轮次进行中提出的旁支问题不会移动轮次起点,同一提示词的重试保留原起点,workflow 在启动时读取一次轮次,所以跨轮继续运行的 workflow 仍按它启动时的那一轮计量。

按次运行的运维上限不变:没有指令时,QWEN_CODE_MAX_TOKENS_PER_WORKFLOW 照旧限制每次运行,spent() 照旧统计本次运行的 agent。运行关于自身的展示在原来的所有位置都保持按次——/workflows、后台任务视图、快照和完成通知报告的是本次运行自己的花费,轮次目标不会被记为这次运行的上限。工具结果在本次运行的数字旁边加上本轮的进度(tokens: 12000 spent by this run · 150000 / 500000 this turn (+500k directive)),用量提示和审批对话框会说明将生效的目标,工具描述和 workflow-authoring skill 讲清轮次语义,并给出"循环到预算用完"和"静态缩放"两种写法。

两条错误信息有改动。被拒绝的派发现在是 Workflow <runId>: token budget exceeded (N / M output tokens). Stopping further agent() calls.;撞到 agent 数量上限时会说明最常见的原因——没设目标时对 budget.remaining() 循环,而这时 remaining()Infinity——以及两种解决办法。

为什么需要

budget 原来只是通过环境变量设置的按次上限。用户无法在消息里说明这一轮可以花多少,脚本也只能凭猜测决定工作量,因为它能读到的唯一数字只统计自己这次运行,而本轮真实的花费——主循环、其他运行、重试——都没有算进去。写作参考早就教了 while (budget.total && budget.remaining() > 50_000),但实际上 budget.total 几乎总是 null,这个循环要么根本不跑,要么在没有守卫时一直跑到 1000 个 agent 的上限。本改动对齐 Claude Code:budget.total 是本轮的 +500k 目标,spent() 是本会话自本轮开始以来的输出 token 增量。

验证方式

启动会话,发送一条以 +500k 结尾的消息,请求一个脚本返回 [budget.total, budget.spent(), budget.remaining()] 的小 workflow。预期 total 为 500000,spent() 已经包含主循环在 workflow 启动前为本轮回答所花的 token;工具结果的运行信息里应分别显示本次运行的花费和本轮的 spent / 500000。同一轮里,用 budget.remaining() > 50_000 守卫循环派发 agent 的脚本应该在达到目标前自行停下。

不带指令发送同样的请求,确认行为不变:budget.totalnull,运行信息为 tokens: N spent (no cap)。设置 QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=1000 且不带指令时,运行信息仍为 tokens: N / 1000 spent,上限照旧按次生效;两者都设时以指令为准。

对带指令的运行打开 /workflows,确认显示的是本次运行自己的 token 花费且没有上限,而不是本轮的数字。只把 +900k 放在 @ 引用的文件里或反引号里,确认不会设定目标。

单元测试命令见英文部分,本地(Linux,Node 22)11 个测试文件共 1333 个测试通过,改动文件的 ESLint 和 Prettier 检查通过,core 包类型检查通过。账本测试驱动真实的 LoggingContentGenerator,遥测路径上不做任何 mock,证明主循环调用和子代理调用都会计入轮次预算所依据的总量。

证据(前后对比)

N/A —— 没有 TUI 布局变化。受影响的是脚本的 budget 全局对象、模型可见的运行信息和错误信息,以及一次性用量提示的文字。

风险与范围

  • 主要风险/取舍:既没有指令也没有环境变量上限时,budget.spent() 现在报告本轮花费而不是本次运行的花费,因此把 budget.spent() 当作"本次运行花了多少"来记录的已保存 workflow 会读到更大的数字。这种情况下 budget.total 仍为 null,不会拦截任何调用,harness 展示本次运行花费的所有位置也仍是按次的。环境变量上限的按次含义完全不变。
  • 主要风险/取舍:内联兜底工具描述(仅在模型无法加载 workflow-authoring skill 时使用)的尺寸上限从 24,000 提到 25,000 字符。main 上这段描述已经是 23,973 字符,任何关于轮次预算的说明都放不进旧上限;skill 增加约 740 字符,兜底描述现在约 24,700 字符。
  • 主要风险/取舍:轮次目标和现有上限一样是软上限——已经在运行的 agent 会继续花费,主循环也在同时花费,所以一轮可能略微超出目标。skill 里写明了这一点。
  • 未验证/不在范围:指令文字保留在模型看到的消息里,不做删除。指令不会延续到下一轮。除消息外没有预算相关的设置或界面。registry、快照和 /workflows 不增加轮次级计数。
  • 破坏性变更/迁移说明:接口层面没有。上述两条错误信息措辞变化;预算错误仍保留 run id 和两个数字。

关联 issue

Part of #11013

A `+500k`-style directive in the user's message now sets the turn's
output-token target. Inside a workflow, budget.total is that target and
budget.spent() counts every output token the session has been charged
since the turn began, the main loop and every agent included, so a
script can size its fan-out to what the turn may cost. Without a
directive, QWEN_CODE_MAX_TOKENS_PER_WORKFLOW keeps capping each run
exactly as before.

The turn opens in LlmClient.sendMessageStream, which every front end
goes through. The directive is read from the user's own text, with
system reminders, @-referenced file and MCP resource content, and code
stripped; only a user query or its retry can carry one. A side question
does not move the turn, a retry keeps its starting point, and a run
reads the turn once at launch.

The run registry, /workflows, snapshots and the completion notification
keep per-run figures; the tool result adds the turn's standing beside
the run's own spend. The budget error keeps the run id, and the
agent-cap error now explains the loop on budget.remaining() with no
target set.

Part of QwenLM#11013
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on ed4d3cb and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— ed4d3cb 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required heading is present, the Risk & Scope bullets are filled in honestly (including the spent() semantics change and the fallback-description size bump), and the Chinese section tracks the English one paragraph for paragraph.

Problem: real and observed, not theoretical hardening. I checked the motivating claim against main rather than taking it on faith: workflow-budget.ts documents the loop pattern while (budget.total && budget.remaining() > 50_000), and the bundled workflow-authoring skill tells authors to read budget.total before committing to a large fan-out — but the only thing that can make total non-null today is QWEN_CODE_MAX_TOKENS_PER_WORKFLOW. So the documented pattern is dead on arrival for anyone who has not set an env var, and remaining() is Infinity for everyone else. #11013 (open, status/in-progress) names "budget" as one of the remaining gaps against the reference implementation, and this PR is a non-closing part of it.

Direction: aligned. This makes an already-documented contract actually usable rather than adding a new surface. Two areas the gate normally escalates on are touched, so here is what I verified instead of escalating blind:

  • telemetry — the change is one additive, read-only getter (getTotalOutputTokens) summing the tokens.candidates that uiTelemetry already records. Nothing about what is recorded, emitted or exported changes.
  • sandbox — the two new WorkflowBudget members are host-only and cannot reach the script realm: the vm bridge is built from an explicit allowlist of fields (workflow-sandbox.ts:1098-1105), not by spreading the budget object, so the "a malicious workflow cannot inflate or deflate the budget" invariant still holds.

Size: core paths, feat type → no hard block. Production 597 lines / test 777 / generated-schema 0 (of 1269 + 105). The 597 crosses the 500-line maintainer-awareness threshold, but I am not escalating it: the author has write access and is the listed owner of packages/core/src/agents/, config/ and goals/ in .github/issue-owners.json — exactly the paths touched — so this is a maintainer-authored change in their own area, which AGENTS.md exempts from the two-tier core gate. Naming the number so the call is visible rather than silent. Below the 1000-line large-PR advisory.

Approach: the scope feels right, and it is better than what I would have written. My own baseline was to repoint spent() at the turn ledger outright — which would have quietly changed what the run registry, /workflows, snapshots and the completion notification report. The runSpent() / runCap() split keeps every per-run surface per-run and puts the turn's figures beside them instead of in place of them.

I also wondered whether the five-regex strip pipeline could be replaced by the existing SendMessageOptions.submittedPrompt ("user-submitted text captured before prompt expansion"), which would avoid re-deriving the user's own words. It cannot, usefully: that field is optional and unevenly populated — Web Shell and the TUI set it, three internal turn sites in client.ts pass undefined explicitly, and I found no headless producer — so a directive would silently stop working outside the interactive front ends. One uniform parse path is the simpler answer even though it costs the strippers.

No drive-by refactors or formatting churn; every file earns its place. HARD_MAX_TOKENS_CEILING = MAX_TURN_BUDGET_TOKENS collapses a duplicated 100M constant instead of adding a second one that can drift.

Risk: no elevated risk signals — none of the 21 files match the revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必需标题都在,Risk & Scope 各条填写得很实在(包括 spent() 语义变化和兜底描述尺寸上限的调整),中文部分与英文逐段对应。

问题: 是真实存在、可观测的问题,不是理论性加固。我对着 main 核实了这个 PR 的动机,而不是直接采信:workflow-budget.ts 里写着 while (budget.total && budget.remaining() > 50_000) 这个循环范式,内置的 workflow-authoring skill 也让作者在大扇出前先读 budget.total —— 但目前唯一能让 total 非 null 的东西只有 QWEN_CODE_MAX_TOKENS_PER_WORKFLOW。也就是说,没设环境变量的用户面前这个范式从落地那天起就是死的,remaining() 对其他人恒为 Infinity#11013(open,status/in-progress)把 "budget" 列为与参考实现之间待补齐的缺口之一,本 PR 是它的非关闭式组成部分。

方向: 对齐。它是把一份已经写进文档的契约变成真正可用,而不是新增一层表面。本 PR 触及了两个通常会触发升级的领域,所以我没有盲目升级,而是逐一核实:

  • telemetry —— 改动只有一个新增的只读取值方法(getTotalOutputTokens),累加的是 uiTelemetry 本来就在记录的 tokens.candidates。记录什么、上报什么、导出什么都没变。
  • sandbox —— 新增的两个 WorkflowBudget 成员只在宿主侧,进不了脚本领域:vm 桥接是按字段白名单显式构造的(workflow-sandbox.ts:1098-1105),不是把 budget 对象整体展开,因此"恶意 workflow 无法抬高或压低预算"这条不变量仍然成立。

规模: 触及核心路径,类型为 feat → 不触发硬性拦截。生产代码 597 行 / 测试 777 行 / 生成与 schema 0 行(总计 1269 + 105)。597 超过了 500 行的"需维护者关注"阈值,但我没有升级:作者拥有 write 权限,并且在 .github/issue-owners.json 中正是 packages/core/src/agents/config/goals/ —— 也就是本 PR 触及的路径 —— 的登记 owner,因此属于维护者在自己负责领域内的改动,AGENTS.md 对这类 PR 豁免两级门禁。把数字写出来,是为了让这个判断可见,而不是悄悄跳过。未达 1000 行的大 PR 提示线。

方案: 范围合理,而且比我自己的写法更好。我的初始方案是直接把 spent() 改指向轮次账本 —— 那会悄悄改变 run registry、/workflows、快照和完成通知所报告的含义。本 PR 用 runSpent() / runCap() 把两者拆开,让所有按次展示的界面继续按次,把轮次数字放在它们旁边而不是取代它们。

我也考虑过:那五条正则的剥离流水线能不能换成已有的 SendMessageOptions.submittedPrompt("prompt 展开前捕获的用户提交文本"),这样就不必反推用户自己写的话。结论是不行:该字段是可选的,而且各前端填充得不一致 —— Web Shell 和 TUI 会填,client.ts 里三个内部轮次入口显式传 undefined,headless 路径我没找到任何生产者 —— 于是指令会在交互式前端之外静默失效。统一走一条解析路径才是更简单的答案,代价就是这几条剥离规则。

没有顺手重构,也没有格式化噪音,每个文件都对得起它的位置。HARD_MAX_TOKENS_CEILING = MAX_TURN_BUDGET_TOKENS 把重复的 1 亿常量收敛成一个,避免将来两处漂移。

风险: 无升级风险信号 —— 21 个文件都不匹配与回滚相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at ed4d3cb9b52ef293f1896744db13e13f893d19a8 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Code review

No critical blockers. The design holds up under reading, and the tests pin the semantics that matter rather than restating the implementation. Three non-blocking items:

1. WorkflowBudgetImpl.fromEnv() is now dead production code. Its only production caller was workflow-runner.ts:175, which this PR switches to fromConfig(). After that the only remaining callers are its own two cases in workflow-budget.test.ts. I grepped the whole of packages/ — it is not re-exported from any core barrel and nothing in packages/cli or the channels reaches it. Worth either dropping it (the tests can go through fromConfig with a config that has no turn support, which is already a covered case) or keeping it deliberately with a note about who it is for. Right now it reads as leftover scaffolding, which is the shape AGENTS.md asks us not to accumulate.

2. The new agent-cap message advises something its reader cannot do. WorkflowAgentCapExceededError now ends with "Add a hard iteration cap to the loop, or pass a token budget." The first half is right and actionable. The second is not: this string reaches the model through tool_result, and the model has no lever to pass a token budget — WORKFLOW_PARAM_SCHEMA has no token or budget property, and the script-side budget global has no setter by design. The real levers belong to someone else (the user typing a +Nk directive, or an operator setting the env var). I mention it only because the sibling error in the same file carries an explicit prior policy about exactly this — P5 R2 (#14) stripped WorkflowBudgetExceededError's advisory tail so model-visible budget text stays factual and does not coach the model toward a knob it cannot turn. Suggest naming the actual levers or dropping the second clause.

3. The strip markers duplicate string literals owned by other modules, with no pointer back to them. I verified all three families match their producers today — --- Content from referenced files --- / --- End of content --- against tools/readManyFiles.ts:113-114, and the MCP delimiters against tools/mcp-resource-content.ts:123,126, where the nonce back-reference correctly resists the smuggling case that file documents at line 61. But the producer constants are module-private, so nothing tells a future editor of readManyFiles.ts that turn-budget.ts depends on the exact wording. If a marker ever changes, the stripper rots silently and a +500k sitting inside an @-referenced file starts setting turn budgets — a quiet failure with no test positioned to catch it. A comment naming the producer files, or sharing the constants, would make the coupling discoverable.

What I checked that a reviewer would reasonably worry about, since these are the load-bearing risks and none of them are visible from the diff alone:

  • Do subagent tokens really land in the same bucket? Yes. Telemetry records through uiTelemetryService.addEvent(evt, config.getSessionId()), and subagent configs come from deriveConfigObject.create(base) with no sessionId override — so they inherit the parent's session id. workflow-budget-ledger.test.ts proves it end to end with the real LoggingContentGenerator and nothing mocked on the telemetry path: 1000 tokens before the turn is excluded by the baseline, then a 300-token main-loop call and a 700-token subagent call both land in spent(), while runSpent() correctly stays at 0.
  • Is the isConcurrentSideQuery guard in client.ts dead defensive code? No, it is load-bearing. startsInteraction (client.ts:2880) is true for any UserQuery including a concurrent side query — the side-query exclusion lives only in the separate goal-proposal block at 3204. Without the added guard a side question would move the running turn's baseline and drop its target. The client.test.ts case pins this.
  • Is one snapshot per Config safe across session switches? Yes, because current(sessionId) fails closed: on a /resume, /branch or ACP rotation (Config.startNewSession) the stale snapshot is rejected and the run falls back to the env cap or null. The failure mode is "directive not applied", never "another session's target applied".
  • Does the new turnSpent / turnTotal in the display payload risk a type error? No — safeStringifyDisplayPayload(payload: unknown), and WorkflowToolResult does not declare tokens.
  • Does the ?.() chaining on Config accessors violate "no handling for impossible scenarios"? It matches the existing convention here (config.getWorkflowRunRegistry?.() in the same runner file) and is exercised by the "config with no turn support at all" case.

The disclosed behaviour change is scoped correctly: with no directive and no env cap, spent() now reports the turn, but total stays null so remaining() stays Infinity and nothing gates — the only way it bites is a saved script that reads spent() without guarding on total, which Risk & Scope says outright.

sequenceDiagram
    participant P1 as User
    participant P2 as LlmClient
    participant P3 as TurnBudget
    participant P4 as UiTelemetry
    participant P5 as WorkflowRunner
    participant P6 as WorkflowBudget
    participant P7 as Script
    P1->>P2: message ending in a +500k directive
    P2->>P2: strip reminders, at-refs and code
    P2->>P4: getTotalOutputTokens(sessionId)
    P4-->>P2: baseline, e.g. 1234
    P2->>P3: beginTurn(target 500000, baseline)
    P1->>P5: workflow tool call, same turn
    P5->>P3: current(sessionId)
    P3-->>P5: snapshot, or null on mismatch
    P5->>P6: fromConfig, total 500000, source directive
    P7->>P6: budget.remaining() before agent()
    P6->>P4: getTotalOutputTokens(sessionId)
    P4-->>P6: 151234 charged, main loop included
    P6-->>P7: 348766 left, so the dispatch is admitted
Loading
Files changed (21)
File What changed
packages/core/src/core/turn-budget.ts New. Directive parser, the request-stripper that isolates the user's own words, and the one-snapshot-per-Config TurnBudget whose current() fails closed on a session mismatch.
packages/core/src/core/turn-budget.test.ts New. Parser cases plus the stripper: directives in reminders, at-refs and code do not count.
packages/core/src/core/client.ts Opens the turn inside startsInteraction, skipping concurrent side queries; retries of the same prompt keep their baseline.
packages/core/src/core/client.test.ts Pins all four turn-opening rules: directive wins over a prepended reminder, cron gets none, tool result and side query leave the turn alone, retry keeps the starting point.
packages/core/src/telemetry/uiTelemetry.ts Additive read-only getter summing per-model tokens.candidates for a session.
packages/core/src/telemetry/uiTelemetry.test.ts Covers the new getter, including the empty-bucket case.
packages/core/src/agents/runtime/workflow-budget.ts The core change: total gains a source, spent() reads the turn unless the source is an env cap, and runSpent() / runCap() preserve per-run meaning. Rewritten fileoverview.
packages/core/src/agents/runtime/workflow-budget.test.ts Source semantics, fromConfig precedence, turn read once at launch, foreign-session turn ignored.
packages/core/src/agents/runtime/workflow-budget-ledger.test.ts New. The real LoggingContentGenerator with nothing mocked on the telemetry path — the evidence that main-loop and subagent tokens share one ledger.
packages/core/src/agents/runtime/workflow-orchestrator.ts The registry emitter now mirrors per-run figures via a small helper instead of spent() / total.
packages/core/src/agents/runtime/workflow-orchestrator.test.ts Asserts the emitter carries run spend and no cap under a turn target.
packages/core/src/agents/runtime/workflow-runner.ts Builds the budget from config, and registers runCap() so a turn target is never recorded as this run's cap.
packages/core/src/agents/runtime/workflow-runner.test.ts Pins that a directive run registers a null per-run cap.
packages/core/src/agents/runtime/workflow-sandbox.ts Two optional host-only members on the budget interface, documented as never bridged into the script.
packages/core/src/agents/workflow-run-registry.ts Doc-comment correction: the mirrored figure is per-run spend.
packages/core/src/agents/runtime/workflow-agent-failure.ts Agent-cap message now explains the usual cause — see finding 2.
packages/core/src/config/config.ts Owns the TurnBudget instance and exposes it; the comment records that derived Configs share it through the prototype on purpose.
packages/core/src/tools/workflow/workflow.ts Tool result, run trailer, usage banner and tool description all distinguish the run's own figures from the turn's.
packages/core/src/tools/workflow/workflow.test.ts Covers the three banner shapes and the trailer wording; raises the fallback-description length assertion from 24,000 to 25,000.
packages/core/src/skills/bundled/workflow-authoring/SKILL.md New "Scaling to the token budget" section with the loop-until-budget and static-scaling patterns.
packages/core/src/skills/bundled/workflow-authoring/SKILL.test.ts Anchor assertions for the new section.

Test evidence

This is an unattended CI run, so per the gate's rules I did not build or execute any PR-derived code. The evidence below is the PR's own CI on the reviewed commit, read through the API. The two checks that matter most for this diff — the Linux unit suite and lint/typecheck — were still running when this comment was posted; the Qwen Triage Finalize job rewrites the table region in place once they settle and performs any deferred approval.

Final CI results for ed4d3cb (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

No check is red, so there is no failing-job log to quote. 56 further check-runs on this commit are skipped bot-orchestration jobs (ack-review-request, resolve-pr, publish-verify, tmux-testing and similar), not CI signal, and review-pr was still queued. The macOS and Windows unit jobs are skipped by the workflow, which lines up with the author's own Tested-on table marking Linux only.

Not verified, and the reason: the author's "1333 tests pass locally" and "ESLint, Prettier and typecheck clean" are the author's claim, not evidence I re-ran — this gate never executes PR code, and the unit suite above had not finished. The claimed fallback-description growth to roughly 24,700 characters against a 25,000 assertion is likewise the author's measurement; I confirmed only that the assertion moved from 24,000 to 25,000 and that the cap lives in the test, not in a production constant.

Sandboxed verification would settle what CI cannot: @qwen-code /verify — that a +500k directive typed into a real prompt actually reaches budget.total inside a running workflow, and that spent() includes the main loop's own tokens end to end. The ledger test proves the telemetry half in isolation, but nothing in the suite drives prompt → beginTurnBudgetWorkflowRunner.startfromConfig as one path, so a wiring mistake between those four would pass green. @qwen-code /tmux is the lane for the other half: the usage banner and the tokens: N spent by this run · M / T this turn trailer are user- and model-visible text that no unit test renders.

中文说明

代码审查: 没有阻断性问题。设计经得起读,测试钉住的是语义而不是把实现复述一遍。三条非阻断意见:

  1. WorkflowBudgetImpl.fromEnv() 现在成了死的生产代码。 它唯一的生产调用点是 workflow-runner.ts:175,本 PR 把它换成了 fromConfig();此后只剩 workflow-budget.test.ts 里自己的两个用例在调。我在整个 packages/ 下搜过,它没有从任何 core barrel 再导出,packages/cli 和各 channel 也都没有用到。建议要么删掉(测试可以走"无轮次支持"的 config,那已经是覆盖到的用例),要么明确保留并注明给谁用。现在它读起来像遗留脚手架,正是 AGENTS.md 要求不要积累的形状。

  2. 新的 agent 上限报错建议了一件读者做不到的事。 WorkflowAgentCapExceededError 结尾是"给循环加一个硬性迭代上限,或者传入一个 token 预算"。前半句对且可执行;后半句不行:这段文字通过 tool_result 到达模型,而模型没有任何手段传入 token 预算 —— WORKFLOW_PARAM_SCHEMA 没有 token 或 budget 参数,脚本侧的 budget 全局对象也按设计没有 setter。真正的开关在别人手里(用户输入 +Nk 指令,或运维设置环境变量)。之所以提这条,是因为同一个文件里的姊妹错误对这件事有明确的既定策略 —— P5 R2 (为什么不能跟cc或者geminicli一样,把密钥放在终端的环境变量里面? #14) 特意删掉了 WorkflowBudgetExceededError 的建议性尾巴,让模型可见的预算文本只陈述事实,不去引导模型碰它转不动的旋钮。建议改成点名真实的开关,或者删掉后半句。

  3. 剥离用的标记复制了别的模块拥有的字符串字面量,且没有回指。 我核实过三类标记目前都与生产者一致 —— --- Content from referenced files --- / --- End of content --- 对应 tools/readManyFiles.ts:113-114,MCP 分隔符对应 tools/mcp-resource-content.ts:123,126,其中 nonce 反向引用正确防住了该文件第 61 行描述的夹带情形。但生产者的常量是模块私有的,将来改 readManyFiles.ts 的人不会知道 turn-budget.ts 依赖这个确切措辞。一旦标记变化,剥离逻辑会静默失效,@ 引用文件里的 +500k 就会开始设定轮次预算 —— 一种安静的失败,而且没有测试正好卡在这个位置。加一条注明生产者文件的注释,或者共享常量,都能让这层耦合可被发现。

我替审阅者核实过的关键点(这些是真正吃重的风险,且单看 diff 看不出来):

  • 子代理的 token 真的进同一个桶吗? 是。遥测通过 uiTelemetryService.addEvent(evt, config.getSessionId()) 记录,而子代理的 config 来自 deriveConfig —— 即 Object.create(base) 且没有覆盖 sessionId —— 所以继承父级的 session id。workflow-budget-ledger.test.ts 用真实的 LoggingContentGenerator、遥测路径上不做任何 mock,端到端证明了这点:轮次开始前的 1000 token 被基线排除,随后主循环的 300 和子代理的 700 都进入 spent(),而 runSpent() 正确地保持为 0。
  • client.ts 里那个 isConcurrentSideQuery 判断是多余的防御吗? 不是,它吃重。startsInteraction(client.ts:2880)对任何 UserQuery 都为真,包括并发旁路提问 —— 排除旁路提问的逻辑只存在于 3204 行那个独立的 goal-proposal 分支里。没有这个判断,一次旁路提问就会移动进行中轮次的基线并丢掉它的目标。client.test.ts 有对应用例钉住。
  • 每个 Config 只存一份快照,跨会话切换安全吗? 安全,因为 current(sessionId) 是失败即关闭的:/resume/branch 或 ACP 会话轮换(Config.startNewSession)之后旧快照会被拒绝,运行退回环境变量上限或 null。失败模式是"指令不生效",绝不会是"套用了别的会话的目标"。
  • 展示载荷里新增的 turnSpent / turnTotal 有类型风险吗? 没有 —— safeStringifyDisplayPayload(payload: unknown),且 WorkflowToolResult 并未声明 tokens
  • Config 取值方法用 ?.() 是否违反"不为不可能的场景做处理"? 这与该处既有惯例一致(同一个 runner 文件里的 config.getWorkflowRunRegistry?.()),并且被"完全不支持轮次的 config"那个用例覆盖到。

已披露的行为变化范围划得对:既无指令也无环境变量上限时,spent() 改为报告轮次花费,但 total 仍为 nullremaining() 仍是 Infinity,不拦截任何调用 —— 唯一会受影响的是那种读了 spent() 却没有用 total 做守卫的已保存脚本,Risk & Scope 里已直说。

测试证据: 这是无人值守的 CI 运行,按门禁规则我没有构建或执行任何来自本 PR 的代码,下面的证据是本 PR 自己在被审查 commit 上的 CI,通过 API 读取。对这个 diff 最关键的两项 —— Linux 单元测试与 lint/typecheck —— 在本条评论发出时仍在运行Qwen Triage Finalize 任务会在它们结束后就地改写表格区域,并执行任何被推迟的批准。

没有任何检查是红的,所以没有失败日志可引。该 commit 上另有 56 项 check-run 是 skipped 的机器人编排任务(ack-review-request、resolve-pr、publish-verify、tmux-testing 等),不是 CI 信号;review-pr 当时还在排队。macOS 与 Windows 的单元测试任务被 workflow 跳过,这与作者自己的 Tested-on 表格只勾了 Linux 一致。

未验证项及原因:作者所述"本地 1333 个测试通过""ESLint、Prettier 与类型检查干净"属于作者的说法,不是我复跑出的证据 —— 本门禁从不执行 PR 代码,而上面那轮单元测试当时尚未跑完。兜底描述增长到约 24,700 字符、对应 25,000 的断言,同样是作者的测量;我只确认了断言从 24,000 移到 25,000,以及这个上限位于测试中而非生产常量里。

沙箱验证可以补齐 CI 补不上的部分:@qwen-code /verify —— 验证真实 prompt 里输入的 +500k 指令确实抵达运行中 workflow 的 budget.total,且 spent() 端到端包含主循环自己的 token。账本测试单独证明了遥测那一半,但整套测试里没有任何一处把 prompt → beginTurnBudgetWorkflowRunner.startfromConfig 作为一条完整路径驱动过,因此这四者之间的接线错误会一路绿灯通过。@qwen-code /tmux 对应另一半:用量提示横幅和 tokens: N spent by this run · M / T this turn 这段尾注是用户与模型可见的文本,没有单元测试会把它渲染出来。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at ed4d3cb9b52ef293f1896744db13e13f893d19a8 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean review; the three findings are nits I would not block on, and the one thing keeping this from a 5 is that the unit suite had not finished when I wrote this.

Going back to the proposal I wrote before opening the diff: this PR beats it. I would have repointed spent() at the turn ledger and broken the per-run meaning of the registry, /workflows, snapshots and the completion notification in one move — the kind of change that reads fine in a diff and gets reported as a bug three weeks later by someone whose workflow logged the wrong number. The runSpent() / runCap() split is the part of this PR I would not have thought of, and it is why the disclosed behaviour change stays as narrow as it is: total remains null without a directive, so remaining() remains Infinity and nothing gates.

The instinct I usually have toward a 597-line core feature — that some of it must be speculative — did not survive contact with the diff. There is no config surface, no settings key, no feature flag, no abstraction for a single use. It is a parser, a snapshot holder, one telemetry getter, and a threading change. The one place I expected to find over-reach, the five strip regexes, is instead the place where the author did more than I would have: the MCP delimiter matcher back-references the nonce, which resists the exact smuggling case mcp-resource-content.ts documents. Someone thought about untrusted text rather than just about the happy path.

What actually convinced me was the ledger test. A claim like "every output token this turn, the main loop and every agent included" is easy to write and easy to fake with a mock that returns the number the assertion wants. This one drives the real LoggingContentGenerator with nothing mocked on the telemetry path, charges 1000 tokens before the turn to prove the baseline excludes them, then charges 300 from the main loop and 700 from inside a subagent context and asks for 1000 — while separately asserting runSpent() stayed at 0. I verified the mechanism independently rather than trusting the test: subagent configs come from deriveConfig (Object.create(base), no sessionId override), and telemetry buckets on config.getSessionId(), so the inheritance is what makes the shared ledger true. Test and mechanism agree.

The client.test.ts cases are the other half of why I am comfortable. The four rules that make this safe — a directive in a prepended reminder does not count, cron and goal turns get no target, a concurrent side question does not move a running turn, a retry keeps its baseline — are each pinned, and the side-query one is genuinely load-bearing rather than belt-and-braces: startsInteraction is true for any UserQuery, so without that guard a side question would silently rebaseline the turn in flight.

My remaining hesitation is not about correctness, it is about reach. Everything above is static reading plus unit evidence, and the end-to-end path (a human typing +500k, the directive surviving prompt assembly, a script reading it back) is not driven by anything in CI. That is the gap the /verify and /tmux lines in my previous comment name, and it is a reason for a maintainer to want one of them run before merge — not a reason to hold the PR up now.

If I were maintaining this in six months I would thank the author, with one exception: I would find the strip markers in turn-budget.ts while editing readManyFiles.ts and wonder why nobody told me they were connected. Finding 3 is the only one of the three I would genuinely want addressed, and a comment naming the producer files is enough.

Approving, with the two nits above left to the author's judgement. CI is still running on this commit — the Linux unit suite and lint/typecheck had not finished — so approval is deferred until CI lands green on ed4d3cb9b52ef293f1896744db13e13f893d19a8; the finalize job posts the commit-pinned approval at that point, and withholds it if anything lands red or the head moves.

中文说明

置信度:4/5 —— 审查干净;三条意见都是我不会据此拦截的小问题,唯一让它没到 5 分的原因是我写下这段时单元测试还没跑完。

回到我在打开 diff 之前写下的方案:这个 PR 比我的方案好。我原本会直接把 spent() 改指向轮次账本,一步就把 registry、/workflows、快照和完成通知的按次含义全弄坏 —— 那种改动在 diff 里看着没问题,三周后由某个记错了数字的 workflow 用户报成 bug。runSpent() / runCap() 的拆分是这个 PR 里我想不到的一手,也正因如此,已披露的行为变化才能收得这么窄:没有指令时 total 仍为 nullremaining() 仍是 Infinity,不拦截任何调用。

面对 597 行核心特性我通常会有的直觉 —— 里面总该有些投机性的东西 —— 在读完 diff 后没能成立。没有配置面,没有设置项,没有特性开关,没有为单一用途造的抽象。它就是一个解析器、一个快照持有者、一个遥测取值方法,加一处串联改动。而我原以为最可能过度设计的地方,也就是那五条剥离正则,反而是作者做得比我更多的地方:MCP 分隔符的匹配回引了 nonce,正好防住 mcp-resource-content.ts 自己记录在案的那种夹带情形。有人认真考虑过不可信文本,而不只是顺利路径。

真正说服我的是账本测试。"本轮每一个输出 token,含主循环与所有 agent"这种断言很好写,也很好用一个返回期望值的 mock 糊弄过去。而这个测试驱动真实的 LoggingContentGenerator,遥测路径上不做任何 mock:先在轮次开始之前记入 1000 token 以证明基线会把它们排除,再让主循环记 300、子代理上下文里记 700,然后要求得到 1000 —— 同时另外断言 runSpent() 保持为 0。我没有只信这个测试,而是独立核实了机制:子代理的 config 来自 deriveConfigObject.create(base),未覆盖 sessionId),而遥测按 config.getSessionId() 分桶,正是这层继承让共享账本成立。测试与机制互相印证。

client.test.ts 的用例是我放心的另一半。让这件事安全的四条规则 —— 前置 reminder 里的指令不算、cron 与 goal 轮次没有目标、并发旁路提问不移动进行中的轮次、重试保留原基线 —— 每一条都被钉住了;其中旁路提问那条是真正吃重的,而不是多余的保险:startsInteraction 对任何 UserQuery 都为真,所以少了这个判断,一次旁路提问就会在运行中悄悄重置轮次基线。

我剩下的犹豫不在正确性,而在覆盖范围。上面所有结论都来自静态阅读加单元证据,而端到端那条路径(真人输入 +500k、指令在 prompt 组装中存活、脚本把它读回来)CI 里没有任何东西驱动过。这正是我上一条评论里 /verify/tmux 两行所指的缺口,也是维护者在合并前会想跑其中之一的理由 —— 但不是现在就把这个 PR 压住的理由。

如果六个月后由我来维护这份代码,我会感谢作者,只有一处例外:我会在改 readManyFiles.ts 时发现 turn-budget.ts 里的剥离标记,然后纳闷怎么没人告诉我这两者有关联。三条意见里我真正希望处理的是第 3 条,而加一条注明生产者文件的注释就够了。

同意合并,上面两条小意见留给作者自行判断。该 commit 的 CI 仍在运行 —— Linux 单元测试与 lint/typecheck 当时尚未结束 —— 因此批准推迟到 CI 在 ed4d3cb9b52ef293f1896744db13e13f893d19a8 上全绿之后;届时由 finalize 任务发出绑定该 commit 的批准,若有任何检查变红或 head 发生移动则不予批准。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at ed4d3cb9b52ef293f1896744db13e13f893d19a8 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

LGTM, looks ready to ship — CI landed green after the review. ✅

@qqqys
qqqys enabled auto-merge September 15, 2026 08:00

@yiliang114 yiliang114 left a comment

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.

LGTM — no blocking issues. The runSpent()/runCap() split keeps the per-run registry semantics intact instead of repointing spent() and silently redefining what /workflows and snapshots report. CI green on ed4d3cb with the bot's approval on this head; this adds the second (human) vote. The three open findings are nit-level and fine as follow-ups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants