feat(core,cli): Workflow tool token budget + per-run UI surfacing (P5)#5231
Merged
Conversation
qwen-code-ci-bot
requested changes
Jun 17, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Hi @LaZzyMan, thanks for the detailed PR — the description is thorough and clearly well thought out.
However, the PR body doesn't follow our PR template. The template requires these sections (which are missing):
## What this PR does## Why it's needed## Reviewer Test Plan(with### How to verify,### Evidence (Before & After),### Tested on)## Risk & Scope## Linked Issues<details><summary>中文说明</summary>block
The content is all here — it's just organized under different headings (## Summary, ## What lands, ## Test coverage, ## Test plan, etc.) that don't match the template. Could you restructure the body to use the template headings? Most of the existing content maps directly:
- "Summary" → "What this PR does" + "Why it's needed"
- "Test plan" → "Reviewer Test Plan" (with the sub-sections)
- Add a "Risk & Scope" section and the 中文说明 block
This helps maintainers scan PRs consistently and won't take long given how complete the description already is.
中文说明
感谢详细的 PR 描述！内容很充实，但 PR 正文未使用我们的PR 模板。请将现有内容重新组织到模板要求的标题下（What this PR does / Why it's needed / Reviewer Test Plan / Risk & Scope / Linked Issues / 中文说明）。大部分内容可以直接映射过去，不会花太多时间。
— Qwen Code · qwen3.7-max
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Qwen Code Review — PR #5231
Reviewer: Qwen Code (automated) · Model: qwen3.7-max
Approach: Multi-agent adversarial review (9 parallel agents) + 2-round reverse audit convergence
Summary
This PR adds a per-run output-token budget for the Workflow tool (P5 of Dynamic Workflows port). The implementation is well-structured and the PR description is thorough. Tests cover the happy paths well (175 tests passing). The review identified 4 critical issues and 7 suggestions — the critical items are token-tracking gaps in the orchestrator that could lead to budget gate bypass or silent undercounting.
Findings
Critical (4):
- Schema-mode success path missing
onTokens— structured-output agents never report tokens - Budget gate bypassed by
parallel()batch — overshoot can be N× worse than documented - Failed/timed-out/cancelled agents' tokens never recorded — budget undercounts
- No
debugLoggercall when budget gate rejects — diagnostic gap
Suggestions (7):
5. resolveUsageBanner JSDoc says "BOTH paths" but only called from success
6. perPhaseTokens null sentinel not surfaced in /workflows detail or phase tree UI
7. Token counts as raw integers vs formatTokenCount — inconsistent formatting
8. onBudgetUpdated fires emitStatusChange unconditionally on no-op mutations
9. No test for concurrent fan-out overshoot behavior
10. Capped banner shape (total !== null) never tested
11. Final display vs live display tokens condition inconsistent
Low-confidence finding omitted: duplicated budget chip formatting (only 2 sites, acceptable)
Collaborator
Author
P5 E2E test reportReal-LLM end-to-end verification of the P5 changes against a live Environment
Group 1: tmux real-scenario (P5 surfaces in the live TUI)Started a fresh 1.1 First workflow call — banner appearsVerified:
1.2
|
| Suite | New | Coverage |
|---|---|---|
workflow-budget.test.ts |
18 | resolveMaxTokensPerWorkflow (env unset/empty/non-int/negative/0/clamp), WorkflowBudgetImpl (recordSpent monotonic, remaining() clamps at 0, null total → Infinity, NaN/negative deltas dropped), WorkflowBudgetExceededError (carries runId/budgetTotal/spent, name field) |
workflow-orchestrator.test.ts |
8 | Budget gate exhausted → throw, mid-run overshoot stops at boundary, null total = no gate, no budget = legacy callers unaffected, budgetUpdated emitter fires per success with cumulative spent + total, never on rejection, no fire when budget omitted, subscriber error swallowed |
workflow-run-registry.test.ts |
10 | Register initializes new fields, register seeds tokenBudgetTotal from caller cap, onBudgetUpdated mutates spent + total, per-phase attribution to currentPhase, null-phase sentinel for pre-phase agents, no-op on missing/terminal entries, backwards/zero deltas ignored, fires statusChange, latch one-shot, latch survives reset() |
workflow.test.ts |
4 | Banner appears on first success, banner suppressed by skipWorkflowUsageWarning, failure-path does NOT emit banner or consume latch + status='failed', failed-then-success → banner on the success run |
3.2 CLI surface suites (42 tests)
cd packages/cli && npx vitest run --no-coverage \
src/ui/commands/workflowsCommand.test.ts \
src/ui/components/background-view/BackgroundTasksDialog.test.tsx✓ src/ui/commands/workflowsCommand.test.ts (10 tests)
✓ src/ui/components/background-view/BackgroundTasksDialog.test.tsx (32 tests)
Test Files 2 passed (2)
Tests 42 passed (42)
Of the 42 total, the 4 new P5 tests in workflowsCommand.test.ts:
- List row chips
tokens/capwhen capped (1500/10000t) - List row chips plain spent when uncapped (
500t, no slash) - Detail view renders
tokens/cap/ per-phase chips (· Find · 300t,· Verify · 150t) - Detail view renders
cap: (no cap)when uncapped
3.3 Sibling-drift / regression suites (35 tests)
The memory config_method_test_mock_drift.md recorded that adding a Config method requires stubbing it in every CLI mock — verified no regression by running the suites that exercise mock Configs:
cd packages/cli && npx vitest run --no-coverage \
src/services/BuiltinCommandLoader.test.ts \
src/ui/commands/clearCommand.test.ts \
src/ui/hooks/useResumeCommand.test.ts✓ src/ui/hooks/useResumeCommand.test.ts (9 tests)
✓ src/ui/commands/clearCommand.test.ts (15 tests)
✓ src/services/BuiltinCommandLoader.test.ts (11 tests)
Test Files 3 passed (3)
Tests 35 passed (35)
getSkipWorkflowUsageWarning() is invoked as this.config.getSkipWorkflowUsageWarning?.() (optional call), so a missing method on a mock Config returns undefined → !undefined = true (safe-default banner-on path). Confirmed by these tests passing without any mock updates.
3.4 Typecheck + lint
cd packages/core && npx tsc --noEmit
cd packages/cli && npx tsc --noEmitClean for workflow-related files. (Pre-existing acp-integration/* errors on this branch base — unrelated to P5.)
npx eslint <14 touched files>Clean.
Self-review pass — 2 bugs caught and fixed before push
Adversarial Explore sweep before opening the PR. Two real issues caught in my own implementation:
- "hard ceiling" docstring drift — the cap is a soft gate (overshoots up to
(concurrency_window − 1) × per_dispatch_tokens), but the docstring + banner copy said "hard ceiling". Softened both to "soft cap" with the overshoot bound documented in theworkflow-budget.tsthreat-model block. - Failure-path banner self-inflicted regression — initial attempt fired the banner on the failure path too. Inspecting
coreToolScheduler.ts:801: whenresult.erroris set,createErrorResponsehard-codesresultDisplay = error.message, dropping any customreturnDisplay. The banner would have been invisible AND would have silently consumed the registry latch, causing the next successful run to skip the banner too. Reverted. Failure path no longer touches the latch; new testP5 T7 R1: failure path does NOT emit banner or consume the latchandP5 T7 R1: failed-then-succeeded → banner appears on the SUCCESS runasserts the contract.
Also clarified: QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=0 is treated as unset (no cap) — operators wanting "no workflows at all" should use QWEN_CODE_DISABLE_WORKFLOWS=1.
Coverage summary
Reported by the CI Code Coverage Summary job:
| Package | Lines | Statements | Functions | Branches |
|---|---|---|---|---|
| CLI | 76.47% | 76.47% | 80.35% | 79.74% |
| Core | 82.42% | 82.42% | 84.11% | 84.01% |
(Coverage delta from P5 changes: new workflow-budget.ts is 100% covered by its colocated test file; new WorkflowTool banner / budget paths and registry onBudgetUpdated / shouldShowUsageWarning paths all covered by the 20 new P5 unit tests.)
Collaborator
Author
|
@qwen-code-ci-bot PR body restructured under the template headings ( |
LaZzyMan
added a commit
that referenced
this pull request
Jun 17, 2026
…ish (PR #5231) Addresses 4 Critical + 7 Suggestions from qwen-code-ci-bot's multi-agent review: Critical fixes (orchestrator core): - #1 (workflow-orchestrator.ts): schema-mode success path was missing the onTokens call entirely, so structured-output agents never recorded against the budget. Lifted the token report to a single `reportTokens` helper invoked once after `subagent.execute()` returns, BEFORE the schema/non-schema branch. Both fast-path and override-path dispatch now hit the same reporting site regardless of terminate mode. - #2 (workflow-orchestrator.ts): the entry budget gate in countedDispatch was bypassed by `parallel()` batches — all N thunks fire-check-queue in a single microtask burst with spent=0, so every queued dispatch passed the gate before any could record tokens. Added a SECOND gate inside the limiter.run callback so queued thunks observe budget mutations from already-completed in-flight dispatches at slot-acquire time, restoring the documented overshoot bound of (concurrency_window - 1) × per_dispatch_tokens (previously up to N × per_dispatch_tokens for a single `parallel()` of N items). - #3 (workflow-orchestrator.ts): CANCELLED / TIMEOUT / MAX_TURNS / ERROR terminations threw without recording tokens, so failed dispatches burned budget silently. Same `reportTokens` lift fixes this — tokens are now read before the terminate-mode check on both paths. - #4 (workflow-orchestrator.ts): added debugLogger.warn at both gate sites (entry + intra-limiter) for budget-rejected dispatches. Suggestion fixes: - #5 (workflow.ts): `resolveUsageBanner` JSDoc still said "Called from BOTH the success and failure paths" after the earlier failure-path revert. Corrected to "SUCCESS path only" with the scheduler-override rationale moved into the docstring. - #6 (workflowsCommand.ts, BackgroundTasksDialog.tsx): null-sentinel perPhaseTokens (tokens spent before the first phase() call) was attributed by the registry but never rendered. Detail view + phase tree now surface a "(no phase)" row when the null-key bucket has spend. - #7 (workflowsCommand.ts, BackgroundTasksDialog.tsx): use the existing `formatTokenCount` helper from `cli/ui/utils/formatters.ts` (the same surface statusLinePresets and TurnCard use) so token counts render as `1.5k / 10k` instead of raw integers. - #8 (workflow-run-registry.ts): `onBudgetUpdated` no longer fires `emitStatusChange` when neither tokensSpent nor tokenBudgetTotal changed. Production code fires `budgetUpdated` after every successful dispatch including zero-output-token ones; gating the emit avoids a no-op UI re-render burst on those. - #11 (workflow.ts): final returnDisplay JSON now includes the `tokens` block whenever any usage is reported OR a cap is set, aligned with `buildLivePhaseTreeDisplay` (was only included when spend > 0, inconsistent with the live render). Test additions: - workflow-budget.test.ts: unchanged (18). - workflow-orchestrator.test.ts: +6 R1 tests (parallel-batch overshoot regression for #2, GOAL+CANCELLED/MAX_TURNS/TIMEOUT/ERROR token recording for #3 via createProductionDispatch, schema-mode success token recording for #1, no-onTokens crash safety). Mock subagent extended with getExecutionSummary + nextOutputTokens to drive these. - workflow-run-registry.test.ts: +1 R1 test for #8 emit gating; rewrote the backwards/zero-delta test to the new monotonic-spent contract. - workflow.test.ts: +1 R1 test for #10 (capped banner shape — was untested; only the uncapped shape had coverage). - workflowsCommand.test.ts: +1 R1 test for #6 null-sentinel surfacing. Updated assertions for #7 formatTokenCount output (`1.5k/10kt`). Total: 282 core tests passing (+10 R1), 43 CLI tests passing (+1 R1), 0 lint, 0 typecheck for workflow-touching files. Real-LLM tmux + JSON E2E reconfirmed end-to-end (banner now says "soft cap", display payload shape unchanged, run registers + completes cleanly). #9 fold: the parallel-batch overshoot test serves as the regression guard for the intra-limiter gate fix in #2. #7 partial: workflowsCommand.ts and BackgroundTasksDialog.tsx are the only two `tokens` render sites in P5; both updated. Other token-bearing surfaces (statusLinePresets, TurnCard) already use the helper. PR: #5231
LaZzyMan
force-pushed
the
lazzy/workflow-p5-budget
branch
from
June 17, 2026 08:20
d82c020 to
6c5de81
Compare
Collaborator
Author
P5 Review Round 1 — fixed in
|
| # | Status | Notes |
|---|---|---|
1 — schema-mode success path missing onTokens |
✅ fixed | Lifted token report to a single reportTokens helper invoked once after subagent.execute() returns, BEFORE the schema/non-schema branch. Both fast path and override path now hit the same site. |
2 — budget gate bypassed by parallel() batch |
✅ fixed | Added a SECOND gate inside the limiter.run callback so queued thunks observe budget mutations from already-completed in-flight dispatches at slot-acquire time. Overshoot bound now matches the documented (concurrency_window - 1) × per_dispatch_tokens. Regression test asserts dispatchCalls < N for parallel([N thunks]) with the cap busted mid-flight. |
| 3 — failed/timed-out/cancelled agents' tokens not recorded | ✅ fixed | Same reportTokens lift covers this — tokens are read BEFORE the terminate-mode check on both paths. Added it.each test asserting CANCELLED / MAX_TURNS / TIMEOUT / ERROR each report tokens before throwing. |
4 — no debugLogger call on budget rejection |
✅ fixed | Added debugLogger.warn at both gate sites (entry + intra-limiter). |
Suggestions (7)
| # | Status | Notes |
|---|---|---|
5 — resolveUsageBanner JSDoc drift |
✅ fixed | JSDoc now says "SUCCESS path only" with the coreToolScheduler override rationale inline. |
6 — null-sentinel perPhaseTokens not surfaced |
✅ fixed | /workflows <runId> detail and dialog phase tree now render (no phase) rows when the null-key bucket has spend. |
7 — token counts as raw integers vs formatTokenCount |
✅ fixed | workflowsCommand.ts and BackgroundTasksDialog.tsx now use the existing formatTokenCount helper from cli/ui/utils/formatters.ts (same surface as statusLinePresets and TurnCard). |
8 — onBudgetUpdated no-op emitStatusChange |
✅ fixed | Skip the emit when neither tokensSpent nor tokenBudgetTotal changed. |
| 9 — no test for concurrent fan-out overshoot | ✅ folded into #2 | The new R1 #2: parallel-batch overshoot is bounded by the intra-limiter re-check test is the regression guard. |
| 10 — capped banner shape untested | ✅ fixed | Added test P5 R1 #10: capped banner shape (total !== null) — sets the env override, executes a tool call, asserts the Workflow token cap is N copy AND the absence of the uncapped copy. |
11 — final vs live display tokens inconsistency |
✅ fixed | Final returnDisplay JSON now uses the same condition as buildLivePhaseTreeDisplay (include tokens when ANY usage is reported OR a cap is set). |
Test count delta
- Core workflow suites: 272 → 282 (+10 R1 tests)
- CLI workflow surfaces: 42 → 43 (+1 R1 test)
- All lint + typecheck clean for workflow-touching files
E2E re-confirmation
Real-LLM tmux + headless JSON re-run against qwen3.7-plus post-R1:
- Banner copy now reads "soft cap" instead of "hard ceiling" (matches the actual gate semantics documented in
workflow-budget.ts). - Display payload unchanged on uncapped success path.
- Run still registers + completes cleanly (
wf_125a5ca48834059c → "r1-ok").
The branch was rebased onto current main (004b8bd15) — two commits ahead: 8069e4a84 (original P5) + 6c5de81d8 (this R1).
qwen-code-ci-bot
requested changes
Jun 17, 2026
LaZzyMan
added a commit
that referenced
this pull request
Jun 17, 2026
…ish (PR #5231) Addresses 4 Critical + 7 Suggestions from qwen-code-ci-bot's multi-agent review: Critical fixes (orchestrator core): - #1 (workflow-orchestrator.ts): schema-mode success path was missing the onTokens call entirely, so structured-output agents never recorded against the budget. Lifted the token report to a single `reportTokens` helper invoked once after `subagent.execute()` returns, BEFORE the schema/non-schema branch. Both fast-path and override-path dispatch now hit the same reporting site regardless of terminate mode. - #2 (workflow-orchestrator.ts): the entry budget gate in countedDispatch was bypassed by `parallel()` batches — all N thunks fire-check-queue in a single microtask burst with spent=0, so every queued dispatch passed the gate before any could record tokens. Added a SECOND gate inside the limiter.run callback so queued thunks observe budget mutations from already-completed in-flight dispatches at slot-acquire time, restoring the documented overshoot bound of (concurrency_window - 1) × per_dispatch_tokens (previously up to N × per_dispatch_tokens for a single `parallel()` of N items). - #3 (workflow-orchestrator.ts): CANCELLED / TIMEOUT / MAX_TURNS / ERROR terminations threw without recording tokens, so failed dispatches burned budget silently. Same `reportTokens` lift fixes this — tokens are now read before the terminate-mode check on both paths. - #4 (workflow-orchestrator.ts): added debugLogger.warn at both gate sites (entry + intra-limiter) for budget-rejected dispatches. Suggestion fixes: - #5 (workflow.ts): `resolveUsageBanner` JSDoc still said "Called from BOTH the success and failure paths" after the earlier failure-path revert. Corrected to "SUCCESS path only" with the scheduler-override rationale moved into the docstring. - #6 (workflowsCommand.ts, BackgroundTasksDialog.tsx): null-sentinel perPhaseTokens (tokens spent before the first phase() call) was attributed by the registry but never rendered. Detail view + phase tree now surface a "(no phase)" row when the null-key bucket has spend. - #7 (workflowsCommand.ts, BackgroundTasksDialog.tsx): use the existing `formatTokenCount` helper from `cli/ui/utils/formatters.ts` (the same surface statusLinePresets and TurnCard use) so token counts render as `1.5k / 10k` instead of raw integers. - #8 (workflow-run-registry.ts): `onBudgetUpdated` no longer fires `emitStatusChange` when neither tokensSpent nor tokenBudgetTotal changed. Production code fires `budgetUpdated` after every successful dispatch including zero-output-token ones; gating the emit avoids a no-op UI re-render burst on those. - #11 (workflow.ts): final returnDisplay JSON now includes the `tokens` block whenever any usage is reported OR a cap is set, aligned with `buildLivePhaseTreeDisplay` (was only included when spend > 0, inconsistent with the live render). Test additions: - workflow-budget.test.ts: unchanged (18). - workflow-orchestrator.test.ts: +6 R1 tests (parallel-batch overshoot regression for #2, GOAL+CANCELLED/MAX_TURNS/TIMEOUT/ERROR token recording for #3 via createProductionDispatch, schema-mode success token recording for #1, no-onTokens crash safety). Mock subagent extended with getExecutionSummary + nextOutputTokens to drive these. - workflow-run-registry.test.ts: +1 R1 test for #8 emit gating; rewrote the backwards/zero-delta test to the new monotonic-spent contract. - workflow.test.ts: +1 R1 test for #10 (capped banner shape — was untested; only the uncapped shape had coverage). - workflowsCommand.test.ts: +1 R1 test for #6 null-sentinel surfacing. Updated assertions for #7 formatTokenCount output (`1.5k/10kt`). Total: 282 core tests passing (+10 R1), 43 CLI tests passing (+1 R1), 0 lint, 0 typecheck for workflow-touching files. Real-LLM tmux + JSON E2E reconfirmed end-to-end (banner now says "soft cap", display payload shape unchanged, run registers + completes cleanly). #9 fold: the parallel-batch overshoot test serves as the regression guard for the intra-limiter gate fix in #2. #7 partial: workflowsCommand.ts and BackgroundTasksDialog.tsx are the only two `tokens` render sites in P5; both updated. Other token-bearing surfaces (statusLinePresets, TurnCard) already use the helper. PR: #5231
LaZzyMan
force-pushed
the
lazzy/workflow-p5-budget
branch
from
June 17, 2026 09:27
6c5de81 to
e7393e5
Compare
LaZzyMan
added a commit
that referenced
this pull request
Jun 17, 2026
… dialog coverage (PR #5231) 3 real findings from qwen-code-ci-bot's round 2 review (the other 11 findings on the same review were already addressed by R1 commit 6c5de81 — the bot used a stale snapshot that did not include R1). Fixes: - #12 (workflow.ts): every dispatch completion produced TWO `safeEmitUpdate` calls — once in the `agentCompleted` handler, once in the `budgetUpdated` handler that fires right after. Over a 1000-agent workflow that's 2000 TUI redraws when 1000 suffices. Dropped the `safeEmitUpdate` call from the `agentCompleted` handler and kept it in `budgetUpdated`; the orchestrator fires the two events back-to-back, so the deferred render shows both updates atomically. Production `WorkflowTool.execute()` always wires `WorkflowBudgetImpl.fromEnv()`, so `budgetUpdated` always fires — test paths that omit budget use the injected dispatch shape and don't exercise this emitter wiring. - #14 (workflow-budget.ts): the WorkflowBudgetExceededError message carried an advisory tail — "Increase QWEN_CODE_MAX_TOKENS_PER_WORKFLOW or unset it to remove the cap" — that reaches the LLM via `tool_result`. The model could surface this to the user and effectively coach them to remove the operator-set budget policy. Trimmed to the factual portion only. Operators can still find the env knob via the `debugLogger.warn` at both gate sites that names `MAX_TOKENS_PER_WORKFLOW_ENV` verbatim. - #15 (BackgroundTasksDialog.test.tsx): WorkflowDetailBody had no rendering test coverage. Added 4 cases under a new R2 #15 describe: capped M/N chip with per-phase tally, uncapped plain-spent + zero- chip suppression, hidden chip when both spend and cap are zero/null, and null-sentinel `(no phase)` row. Declined / declined-with-counter-evidence: - #13 (workflow-budget.ts threat-model docstring): bot claimed the overshoot bound is off-by-one — `concurrency_window × per_dispatch` rather than `(concurrency_window - 1) × per_dispatch`. The latter is the correct tighter upper bound: when the gate first tips, the tipping dispatch's own tokens are already counted in `spent`, and only `concurrency_window - 1` other in-flight dispatches remain to add overshoot. The looser bound the bot suggests would mislead operators into oversized safety margins. Test count: 282 → 283 core (+1 R2 #14 negative assertion), 42 → 47 CLI (+5 R2 #15 + null-sentinel coverage). 0 lint, 0 typecheck for workflow-touching files. PR: #5231
Collaborator
Author
P5 Review Round 2 — summaryTriage outcome of the 15 inline findings on the round-2 review: 11 already addressed by R1 commit R2 commit
|
| Finding | Status | Change |
|---|---|---|
#12 — agentCompleted + budgetUpdated doubled UI re-renders |
✅ fixed | Dropped safeEmitUpdate from the agentCompleted handler; budgetUpdated fires immediately after and now drives a single atomic render. Halves UI re-renders per dispatch (1000-agent run: 2000 → 1000 redraws). |
#14 — WorkflowBudgetExceededError advisory tail coaches LLM to disable cap |
✅ fixed | Trimmed message to factual portion only: Workflow <runId> exceeded the token budget (<spent> / <total> output tokens spent).. Env-knob discoverability preserved via debugLogger.warn at both gate sites. Test asserts the trim. |
#15 — WorkflowDetailBody budget chip has no test coverage |
✅ fixed | Added 4 cases under WorkflowDetailBody budget chip (R2 #15): capped M/N + per-phase tally, uncapped plain-spent + zero-chip suppression, hidden chip when both are zero/null, null-sentinel (no phase) row. |
| #13 — overshoot bound claimed off-by-one | ❌ declined | The documented (concurrency_window − 1) × per_dispatch is the correct tighter upper bound: when the gate tips, the tipping dispatch's tokens are already in spent; only concurrency_window − 1 other in-flight dispatches contribute unaccounted overshoot. Full walkthrough in the per-thread reply. |
R1 commit 8e08d699f re-affirmed (11 findings already addressed)
The bot's round-2 review pinned commit d82c0206 (the original P5 commit), missing R1. Each affected thread now has a per-thread reply citing the R1 fix location:
- pre-release: fix ci #1, Where is the config saved? #2, 如何自定义密钥文件 .env可能与其他文件冲突 #3, Are you interested in AI Terminal? #4 (Critical) —
reportTokenslift, intra-limiter gate,debugLogger.warnat gate sites - TypeError in Authentication Selection Interface #5, OpenAI API Error: 401 Incorecct API Key provided #6, API Key是要设成阿里云的API Key吗？ #7, report error when try to auth #8, Use Enter to Set Auth 之后直接崩溃退出 #11 (Suggestion) — JSDoc drift, null-sentinel UI rendering,
formatTokenCountadoption,emitStatusChangegating, display payload consistency - API Error: Streaming setup timeout after 45s #9, 这个怎么调用 Qwen3-Coder，配置界面只有openai #10 (Suggestion) — overshoot regression test + capped banner test
Test count
- Core: 282 → 283 (+1 R2 为什么不能跟cc或者geminicli一样，把密钥放在终端的环境变量里面？ #14 negative assertion)
- CLI: 42 → 47 (+5 R2 运行不了 #15 + R1 OpenAI API Error: 401 Incorecct API Key provided #6 dialog tests)
Branch state
Rebased onto current main (14e6ae8c2); 3 commits ahead:
cb2e02e3aoriginal P58e08d699fR1 (10 R1 tests + R1 fixes)e7393e5bbR2 (this commit)
All 15 threads resolved.
qwen-code-ci-bot
requested changes
Jun 17, 2026
wenshao
requested changes
Jun 17, 2026
Collaborator
Local build + real-test verification (maintainer)Verified PR head Verdict: the feature is solid and works end-to-end in a real binary, and every claimed suite is green. But the one Critical finding from my review — token accounting is bypassed on the 1. Build & test suites — ✅ green (counts even higher than claimed)
2. Real-TUI verification — ✅ user-facing half works in the real binaryBuilt Uncapped session:
Capped session (
3. 🔴 Critical (re-confirmed, still open): token accounting bypassed when
|
| Scenario | onTokens fired? |
|---|---|
| GOAL (execute returns) | [777] ✅ |
| ERROR via return (← the PR's test scenario) | [777] ✅ |
| ERROR via throw (real production path) | [] ❌ leak |
Fix-flip proof: wrapping the call so reportTokens runs in a finally flips the throw-path repro from [] → [777] (the test then fails on the old assertion). So the data is available — getExecutionSummary() is finalized in execute()'s own finally — the dispatch just never reads it on the throw path. The repro is non-vacuous and the fix is effective.
Why it matters: the cap exists to bound runaway runs. A flaky provider (or a script that catches and retries — a pattern the threat-model doc itself calls out) produces a stream of errored dispatches, each burning real output tokens that escape accounting → remaining() stays high → the gate never trips. The cap under-enforces in exactly the error-loop scenario it is designed to bound. CANCELLED / MAX_TURNS / TIMEOUT are unaffected (those return from execute()).
Suggested fix (both dispatch sites):
try {
await subagent.execute(ctx, signal);
} finally {
reportTokens(subagent, opts, onTokens); // valid on the throw path too
}…plus a regression test where the mocked execute throws (not returns) and asserts onTokens still fired.
Standalone repro test (drop into packages/core/src/agents/runtime/ and run with vitest)
const { nextOutputTokens, nextExecuteThrows, nextTerminateMode } = vi.hoisted(() => ({
nextOutputTokens: { value: 0 as number },
nextExecuteThrows: { value: false as boolean },
nextTerminateMode: { value: 'GOAL' as string },
}));
vi.mock('./agent-headless.js', () => ({
AgentHeadless: {
create: async () => ({
execute: async () => {
if (nextExecuteThrows.value)
throw new Error('simulated provider stream error (runReasoningLoop threw)');
},
getFinalText: () => 'ok-text',
getTerminateMode: () => nextTerminateMode.value,
getExecutionSummary: () => ({ outputTokens: nextOutputTokens.value }),
}),
},
ContextState: class { private s: Record<string, unknown> = {};
get(k: string){return this.s[k];} set(k: string,v: unknown){this.s[k]=v;} },
}));
import { createProductionDispatch } from './workflow-orchestrator.js';
// ...3 cases: GOAL→[777], ERROR-via-return→[777], ERROR-via-throw→[] (the leak)4. 🟡 Secondary (Suggestion, unchanged): agentCount gate ordering
agentCount += 1 (:1192) runs before the budget gate (:1216), and the budget-reject return sits above emitter?.agentDispatched?.() (:1232). So budget-rejected calls (a) still consume an agent-count slot, and (b) don't increment the UI's agentsDispatched. A script that catches the budget rejection and keeps calling agent() eventually trips the agentCount > maxAgents guard and throws the wrong terminal error ("exceeded the maximum of N agent() calls") when the real cause is budget exhaustion; agentCount (cap) and agentsDispatched (UI) also diverge. Low severity, but worth a fix while the Critical is being addressed: evaluate the budget gate before the increment, or decrement on reject.
Recommendation
Hold for one small turn: land the finally fix at both dispatch sites + a throw-path regression test (and ideally the agentCount reorder). Both are tiny and local. With those, this is a clean merge — the design, docs, test depth, and the user-facing surfacing are all genuinely good, and I verified the latter works end-to-end in the real binary.
🇨🇳 中文版（完整对应）
本地构建 + 真实测试验证（维护者）
在隔离 worktree 中验证了 PR head e7393e5bba。自我提交 CHANGES_REQUESTED（2026-06-17 14:01 UTC；PR 三个 commit 均为 09:27 UTC）之后没有新 commit 落地，所以这就是我当时标记的那棵树 —— 下面的验证既确认功能可用，也复核我之前那条 Critical 是否仍然成立。
结论： 功能扎实，在真实二进制里端到端可用，所有声称的测试套件全绿。但我 review 里那条 Critical —— execute() 抛错路径上 token 计账被绕过 —— 是真实存在且尚未修复的。 我做了确定性复现，并验证 ~6 行的修复能闭合它。建议合并前先修；其余都没问题。
1. 构建与测试套件 —— ✅ 全绿（数量甚至比声称的更多）
| 步骤 | 结果 |
|---|---|
npm ci |
exit 0 |
npm run build（整个 monorepo 类型检查） |
exit 0 |
| Core 5 个套件 | 283 passed |
| CLI 2 个套件 | 47 passed |
小提示：PR 正文写的是
272/42，HEAD 实际为283/47（+11 / +5，应是 R2 commit 在正文写完后又加了测试）。通过的测试比声称的还多 —— 不是回归，只是描述里的数字陈旧了。
2. 真实 TUI 验证 —— ✅ 用户可见的那半部分在真实二进制里可用
构建 node packages/cli/dist/index.js，在 tmux 中对真实 qwen3.7-max（YOLO）驱动，QWEN_CODE_ENABLE_WORKFLOWS=1。
未设上限的会话：
- 第一次
Workflow调用 —— banner 逐字前置：> Workflows have no per-run token cap. Set `QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<n>` (env) for a soft cap. Suppress this notice with `skipWorkflowUsageWarning: true` in settings. /workflows→Workflow runs (1 total · 0 running)+wf_… completed 2ms smoke · Plan · 1 phase（无预算 chip —— 正确，0 花费 + 无上限）。/workflows <runId>→tokens : 0 cap : (no cap) Phases (1) · Plan- 第二次
Workflow调用 → banner 被抑制（在整段 scrollback 里 grepWorkflows have no per-run token cap计数 = 1）。每会话一次的 latch 得到确认。
设上限的会话（QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=100）：
- banner 切换为「有上限」形态，逐字：
> Workflow token cap is 100 (per `QWEN_CODE_MAX_TOKENS_PER_WORKFLOW`). Suppress this notice with `skipWorkflowUsageWarning: true` in settings. /workflows行内 chip 出预算：wf_… completed 1ms capped · Plan · 1 phase · 0/100t。
3. 🔴 Critical（复核后仍然存在）：execute() 抛错时 token 计账被绕过
这就是我 review 里那条。当前 head 没有修复，我现在有了确定性运行时复现。
问题。 在两个分发点（workflow-orchestrator.ts:359 默认路径、:644 override 路径），reportTokens(...) 都写在 await subagent.execute(...) 的下一行 —— 不在 finally 里。而生产中 ERROR 这个 terminate mode 是靠 execute() 重新抛出达到的：
agent-headless.ts:285-294
} catch (error) {
this.terminateMode = AgentTerminateMode.ERROR;
...
throw error; // ← runReasoningLoop 任何失败都会重抛
}
所以一次分发出错时（重试后的 provider/stream 失败 —— 常见的真实 ERROR 路径），控制流跳过 reportTokens，onTokens 永不触发，budget.spent() 少算了这次分发已经烧掉的 token。
PR 自己的测试 R1 #3: records tokens on ERROR failure path (still throws) 之所以能过，仅仅是因为它的 mock execute 是返回的、同时 getTerminateMode() 给出 ERROR —— 那模拟的是罕见的 !chat 提前返回（agent-headless.ts:224-227），不是生产中产生 ERROR 的那条 reasoning-loop 抛错路径。
确定性复现（独立测试，把 execute() mock 成抛出 —— 真实路径）：
| 场景 | onTokens 是否触发 |
|---|---|
| GOAL（execute 返回） | [777] ✅ |
| ERROR 经返回（← PR 测试模拟的场景） | [777] ✅ |
| ERROR 经抛出（真实生产路径） | [] ❌ 泄漏 |
修复翻转证明： 把这次调用包成让 reportTokens 跑在 finally 里，抛错路径的复现就从 [] → [777]（旧断言随即失败）。说明数据本来就有 —— getExecutionSummary() 在 execute() 自己的 finally 里已经定稿 —— 只是抛错路径上没去读它。复现非空过，修复有效。
为何重要： 这个上限的存在就是为了兜住失控的 run。一个不稳定的 provider（或一个 catch 后重试的脚本 —— 威胁模型文档自己点了这个模式）会产生一串出错的分发，每一次都烧掉真实输出 token 却逃过计账 → remaining() 一直偏高 → 闸门永不触发。上限恰恰在它被设计来兜底的「错误循环」场景下失守。CANCELLED / MAX_TURNS / TIMEOUT 不受影响（那几个是从 execute() 返回的）。
建议修复（两个分发点都改）：
try {
await subagent.execute(ctx, signal);
} finally {
reportTokens(subagent, opts, onTokens); // 抛错路径上也成立
}…再加一条回归测试：mock 的 execute 抛出（不是返回），断言 onTokens 仍触发。
4. 🟡 次要（Suggestion，未变）：agentCount 闸门顺序
agentCount += 1（:1192）跑在预算闸门（:1216）之前，且预算 reject 的 return 位于 emitter?.agentDispatched?.()（:1232）之上。于是被预算拒绝的调用：(a) 仍占掉一个 agent-count 名额；(b) 不会递增 UI 的 agentsDispatched。一个 catch 掉预算拒绝、继续调 agent() 的脚本，最终会触发 agentCount > maxAgents 守卫，抛出错误的终止错误（"exceeded the maximum of N agent() calls"），而真正原因是预算耗尽；同时 agentCount（上限）与 agentsDispatched（UI）发生分歧。严重度低，但趁修 Critical 时一并处理较好：把预算闸门挪到自增之前，或在 reject 时自减。
建议
再走一个小回合：在两个分发点补上 finally 修复 + 一条抛错路径回归测试（最好把 agentCount 顺序也一并调整）。两处都很小、很局部。改完即可干净合并 —— 设计、文档、测试深度、以及用户可见的呈现都确实做得好，而后者我已在真实二进制里端到端验证可用。
Methodology: isolated git worktree off PR head e7393e5bba; npm ci + full npm run build; the exact suites from the PR's test plan; a standalone runtime repro with fix-flip proof; and a real qwen binary driven in tmux against a live model. Assisted by claude-opus-4-8 via Qwen Code.
P5 of the Dynamic Workflows port (#4721): per-run output-token budget for the Workflow tool, wired through the orchestrator dispatch gate, WorkflowRunRegistry, BackgroundTasksDialog phase tree, and the /workflows slash command. Also introduces a one-time usage banner the first time a workflow runs in a session, gated by the skipWorkflowUsageWarning setting. Knobs: QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<int> env, per-run cap skipWorkflowUsageWarning: true setting, suppress banner Budget gate semantics: SOFT cap, not pre-commit reservation. Gate is checked at dispatch entry, so concurrent fan-out (parallel / pipeline) can overshoot by up to (concurrency_window - 1) x per_dispatch_tokens before the first overshoot dispatch throws WorkflowBudgetExceededError. Matches upstream Claude Code 2.1.168 semantics. Operators sizing the cap should subtract the overshoot margin. Implementation: - WorkflowBudgetImpl (workflow-budget.ts) + env resolver with HARD_MAX_TOKENS_CEILING=100M ceiling on the env override. - WorkflowBudgetExceededError carries runId / budgetTotal / spent. - countedDispatch budget gate + onTokens callback feeding budget.recordSpent from getExecutionSummary().outputTokens. - WorkflowOrchestratorEmitter.budgetUpdated event; fires after each successful dispatch, skipped on rejection and when budget is null. - WorkflowTask gains tokensSpent / tokenBudgetTotal / perPhaseTokens fields; WorkflowRunRegistry.onBudgetUpdated attributes deltas to currentPhase at fire time and re-emits statusChange. - WorkflowRunRegistry.shouldShowUsageWarning latch fires once per registry instance; survives reset(). - WorkflowTool wires WorkflowBudgetImpl.fromEnv, threads onTokens into createProductionDispatch, mirrors budget into the registry via the emitter, and prepends the usage banner on the SUCCESS path only. - WorkflowDetailBody + /workflows listing + live phase-tree render budget chip (tokens / cap) and per-phase token totals. Verification (270 + 4 + 4 = 272 core + 42 cli): - workflow-budget.test.ts (18) + workflow-orchestrator.test.ts (+8 P5 + budget-gate + budgetUpdated emitter) - workflow-run-registry.test.ts (+10 P5: budget fields, latch, per-phase attribution, no-op on terminal entries) - workflow.test.ts (+4 P5: banner appears once, suppressed by setting, failure-path latch unchanged, fail-then-success re-emits banner) - workflowsCommand.test.ts (+4 P5: row chip capped/uncapped, detail tokens/cap/per-phase chips) - BackgroundTasksDialog.test.tsx unchanged (32 still pass) - Real-LLM E2E (DashScope qwen3.7-plus): tmux session driving Workflow tool, banner verified in returnDisplay, /workflows shows tokens 0 / cap (no cap) on uncapped run, banner suppression on 2nd run confirmed (latch consumed exactly once). Self-review round 1 fixes: - "hard ceiling" docstring softened to "soft cap" with per_dispatch x concurrency_window overshoot bound documented; banner copy aligned ("soft cap" instead of "hard ceiling"). - Attempted failure-path banner reverted after coreToolScheduler inspection: createErrorResponse hard-codes resultDisplay = error.message whenever result.error is set, so a failure-path banner would have been invisible AND would have silently flipped the registry latch, causing the next successful run to skip the banner too. Failure path now does not touch the latch; failure-path test asserts the fail-then-success run still gets the banner. - skipWorkflowUsageWarning setting placement aligned with skipNextSpeakerCheck sibling under settings.model.*. - QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=0 documented as "treated as unset" with explicit pointer to QWEN_CODE_DISABLE_WORKFLOWS=1 for the "no workflows at all" intent. Refs #4721.
…ish (PR #5231) Addresses 4 Critical + 7 Suggestions from qwen-code-ci-bot's multi-agent review: Critical fixes (orchestrator core): - #1 (workflow-orchestrator.ts): schema-mode success path was missing the onTokens call entirely, so structured-output agents never recorded against the budget. Lifted the token report to a single `reportTokens` helper invoked once after `subagent.execute()` returns, BEFORE the schema/non-schema branch. Both fast-path and override-path dispatch now hit the same reporting site regardless of terminate mode. - #2 (workflow-orchestrator.ts): the entry budget gate in countedDispatch was bypassed by `parallel()` batches — all N thunks fire-check-queue in a single microtask burst with spent=0, so every queued dispatch passed the gate before any could record tokens. Added a SECOND gate inside the limiter.run callback so queued thunks observe budget mutations from already-completed in-flight dispatches at slot-acquire time, restoring the documented overshoot bound of (concurrency_window - 1) × per_dispatch_tokens (previously up to N × per_dispatch_tokens for a single `parallel()` of N items). - #3 (workflow-orchestrator.ts): CANCELLED / TIMEOUT / MAX_TURNS / ERROR terminations threw without recording tokens, so failed dispatches burned budget silently. Same `reportTokens` lift fixes this — tokens are now read before the terminate-mode check on both paths. - #4 (workflow-orchestrator.ts): added debugLogger.warn at both gate sites (entry + intra-limiter) for budget-rejected dispatches. Suggestion fixes: - #5 (workflow.ts): `resolveUsageBanner` JSDoc still said "Called from BOTH the success and failure paths" after the earlier failure-path revert. Corrected to "SUCCESS path only" with the scheduler-override rationale moved into the docstring. - #6 (workflowsCommand.ts, BackgroundTasksDialog.tsx): null-sentinel perPhaseTokens (tokens spent before the first phase() call) was attributed by the registry but never rendered. Detail view + phase tree now surface a "(no phase)" row when the null-key bucket has spend. - #7 (workflowsCommand.ts, BackgroundTasksDialog.tsx): use the existing `formatTokenCount` helper from `cli/ui/utils/formatters.ts` (the same surface statusLinePresets and TurnCard use) so token counts render as `1.5k / 10k` instead of raw integers. - #8 (workflow-run-registry.ts): `onBudgetUpdated` no longer fires `emitStatusChange` when neither tokensSpent nor tokenBudgetTotal changed. Production code fires `budgetUpdated` after every successful dispatch including zero-output-token ones; gating the emit avoids a no-op UI re-render burst on those. - #11 (workflow.ts): final returnDisplay JSON now includes the `tokens` block whenever any usage is reported OR a cap is set, aligned with `buildLivePhaseTreeDisplay` (was only included when spend > 0, inconsistent with the live render). Test additions: - workflow-budget.test.ts: unchanged (18). - workflow-orchestrator.test.ts: +6 R1 tests (parallel-batch overshoot regression for #2, GOAL+CANCELLED/MAX_TURNS/TIMEOUT/ERROR token recording for #3 via createProductionDispatch, schema-mode success token recording for #1, no-onTokens crash safety). Mock subagent extended with getExecutionSummary + nextOutputTokens to drive these. - workflow-run-registry.test.ts: +1 R1 test for #8 emit gating; rewrote the backwards/zero-delta test to the new monotonic-spent contract. - workflow.test.ts: +1 R1 test for #10 (capped banner shape — was untested; only the uncapped shape had coverage). - workflowsCommand.test.ts: +1 R1 test for #6 null-sentinel surfacing. Updated assertions for #7 formatTokenCount output (`1.5k/10kt`). Total: 282 core tests passing (+10 R1), 43 CLI tests passing (+1 R1), 0 lint, 0 typecheck for workflow-touching files. Real-LLM tmux + JSON E2E reconfirmed end-to-end (banner now says "soft cap", display payload shape unchanged, run registers + completes cleanly). #9 fold: the parallel-batch overshoot test serves as the regression guard for the intra-limiter gate fix in #2. #7 partial: workflowsCommand.ts and BackgroundTasksDialog.tsx are the only two `tokens` render sites in P5; both updated. Other token-bearing surfaces (statusLinePresets, TurnCard) already use the helper. PR: #5231
… dialog coverage (PR #5231) 3 real findings from qwen-code-ci-bot's round 2 review (the other 11 findings on the same review were already addressed by R1 commit 6c5de81 — the bot used a stale snapshot that did not include R1). Fixes: - #12 (workflow.ts): every dispatch completion produced TWO `safeEmitUpdate` calls — once in the `agentCompleted` handler, once in the `budgetUpdated` handler that fires right after. Over a 1000-agent workflow that's 2000 TUI redraws when 1000 suffices. Dropped the `safeEmitUpdate` call from the `agentCompleted` handler and kept it in `budgetUpdated`; the orchestrator fires the two events back-to-back, so the deferred render shows both updates atomically. Production `WorkflowTool.execute()` always wires `WorkflowBudgetImpl.fromEnv()`, so `budgetUpdated` always fires — test paths that omit budget use the injected dispatch shape and don't exercise this emitter wiring. - #14 (workflow-budget.ts): the WorkflowBudgetExceededError message carried an advisory tail — "Increase QWEN_CODE_MAX_TOKENS_PER_WORKFLOW or unset it to remove the cap" — that reaches the LLM via `tool_result`. The model could surface this to the user and effectively coach them to remove the operator-set budget policy. Trimmed to the factual portion only. Operators can still find the env knob via the `debugLogger.warn` at both gate sites that names `MAX_TOKENS_PER_WORKFLOW_ENV` verbatim. - #15 (BackgroundTasksDialog.test.tsx): WorkflowDetailBody had no rendering test coverage. Added 4 cases under a new R2 #15 describe: capped M/N chip with per-phase tally, uncapped plain-spent + zero- chip suppression, hidden chip when both spend and cap are zero/null, and null-sentinel `(no phase)` row. Declined / declined-with-counter-evidence: - #13 (workflow-budget.ts threat-model docstring): bot claimed the overshoot bound is off-by-one — `concurrency_window × per_dispatch` rather than `(concurrency_window - 1) × per_dispatch`. The latter is the correct tighter upper bound: when the gate first tips, the tipping dispatch's own tokens are already counted in `spent`, and only `concurrency_window - 1` other in-flight dispatches remain to add overshoot. The looser bound the bot suggests would mislead operators into oversized safety margins. Test count: 282 → 283 core (+1 R2 #14 negative assertion), 42 → 47 CLI (+5 R2 #15 + null-sentinel coverage). 0 lint, 0 typecheck for workflow-touching files. PR: #5231
…-arm budgetUpdated, gate-before-count (PR #5231) 3 fixes for round 3 review. wenshao (human maintainer) caught a real production-path token leak that R1's `R1 #3` test missed; bot also found that R2 #12 (UI emit dedup) left the error arm with zero re-renders. Critical fixes (orchestrator core): - #6 (wenshao): `reportTokens` was on the line AFTER `await subagent.execute(...)` at both dispatch sites (fast path :353 + override path :642) — NOT in a `finally`. `AgentHeadless.execute()` re-throws on real reasoning-loop failure (`agent-headless.ts:287-294`), so the production ERROR path skipped `reportTokens` entirely and the dispatch's burned tokens leaked. Wrapped `await subagent.execute()` in `try { ... } finally { reportTokens(...) }` at both sites. `getExecutionSummary()` is safe to read inside the throw path because `AgentHeadless.execute()`'s own outer `finally` finalizes stats before the throw propagates. R1's `R1 #3` test passed only because the mock execute() RETURNED with ERROR mode (the rare `createChat` early-return); the production reasoning-loop throw was untested. Test pattern: R3 #6 tests now mock `execute()` to THROW directly, asserting `onTokens` still fires for both fast path and override path (sibling-drift coverage). - #1 (bot): with R2 #12's UI-emit dedup, the error arm of `countedDispatch` fired `agentCompleted` (no `safeEmitUpdate`) and NEVER fired `budgetUpdated` — producing ZERO UI re-renders per failed dispatch. The registry's `tokensSpent` / `perPhaseTokens` also diverged from `budget.spent()` because the host counter advanced (via the reportTokens-in-finally above) while the registry never saw it. Error arm now also fires `emitter?.budgetUpdated?.()` with the post-throw spent + total. Updated R1's "does NOT fire on dispatch rejection" test to assert the new contract (DOES fire, with the cumulative spent) — that test only passed before because the mock dispatch threw without ever calling `budget.recordSpent`, masking the production behavior. - #7 (wenshao, suggestion → accepted): `agentCount += 1` ran BEFORE the budget gate. After budget exhaustion, every subsequent `agent()` call still incremented `agentCount`, eventually tripping the agent-cap and surfacing the WRONG terminal error (`Workflow exceeded the maximum of N agent() calls per run`) when the real cause was budget exhaustion. Moved the budget gate above the `agentCount += 1`. Also keeps `agentCount` and `agentsDispatched` (registry counter) counting the same set of calls. New test loops 1100 budget-rejected dispatches and asserts the script completes with budget errors only, never agent-cap errors. Declined (round 5 Suggestion bar): - bot #2 (rename `shouldShowUsageWarning` → `tryConsumeUsageWarning`): naming style, R5 → overthinking. - bot #3 (debugLogger on NaN drop in recordSpent): hostile-provider defensive hardening, R5 → overthinking. - bot #4 (debugLogger on negative delta in onBudgetUpdated): same. - bot #5 (triple emitStatusChange per dispatch): efficiency, R5 → overthinking; #1 fix kept the dispatched / completed / budget callback shape, and TUI emits are still 2 per dispatch (the middle one no longer fires safeEmitUpdate per R2 #12). Declined with counter-evidence: - (None this round.) Test count: 283 → 287 core (+4 R3 tests: throw-path fast/override, budgetUpdated on error, agentCount/gate ordering), 47 CLI unchanged. 0 lint, 0 typecheck for workflow-touching files. CI lint failure on this PR is pre-existing main breakage (shellcheck SC2295 in `.github/workflows/qwen-autofix.yml:598` introduced by commit a335f9c, unrelated to this PR's diff) — leaving alone. PR: #5231
LaZzyMan
force-pushed
the
lazzy/workflow-p5-budget
branch
from
June 18, 2026 03:06
e7393e5 to
8598e4b
Compare
Collaborator
Author
P5 Review Round 3 — summary3 fixed ( Fixed in commit
|
| # | Reviewer | Finding | Fix |
|---|---|---|---|
| #6 | wenshao (Critical) | reportTokens bypassed when subagent.execute() THROWS — production ERROR re-throws from AgentHeadless.execute() itself (not from the orchestrator's terminate-mode gate). R1 #3 masked this by mocking execute() to RETURN with ERROR mode (the rare createChat early-return path). |
Wrapped both dispatch sites in try { await subagent.execute() } finally { reportTokens(...) }. New tests mock execute() to THROW directly, covering fast path (workflow-orchestrator.ts:353) and override path (:642) sibling-drift. |
| #1 | qwen-code-ci-bot (Critical) | After R2 #12 (UI emit dedup), the error arm fired agentCompleted with NO safeEmitUpdate and never fired budgetUpdated — producing zero UI re-renders per failed dispatch. Combined with R3 #6 above, budget.spent() now advances on the throw path, so the registry's tokensSpent would diverge from the host budget without this fire. |
Error arm now also fires emitter?.budgetUpdated?.(budget.spent(), budget.total) gated on if (budget). Updated the does NOT fire on rejection test to assert the new contract (DOES fire). |
| #7 | wenshao (Suggestion → accepted) | agentCount += 1 ran BEFORE the budget gate. After budget exhaustion, every subsequent agent() call still incremented agentCount and eventually tripped the agent-cap, surfacing the wrong terminal error (Workflow exceeded the maximum of N agent() calls per run). |
Moved budget gate above agentCount += 1. New test loops 1100 budget-rejected calls with cap=100 and asserts no agent-cap error surfaces. Cheap fix tied to the same code site as #6; accepted despite round 5 Suggestion bar because the cost-benefit is clean. |
Declined (round 5 Suggestion bar)
| # | Finding | Reason |
|---|---|---|
| #2 | rename shouldShowUsageWarning() → tryConsumeUsageWarning() |
Style/naming, R5 → overthinking. |
| #3 | debugLogger.warn on NaN/Infinity in recordSpent |
Hostile-provider defensive hardening, R5 → overthinking. AgentHeadless already logs stats-finalization issues. |
| #4 | debugLogger.warn on negative delta in onBudgetUpdated |
Same as #3. recordSpent is monotonic, so negative deltas can only arrive from a host-side bug; silent no-op is the right safety choice. |
| #5 | Triple emitStatusChange per dispatch |
Efficiency, R5 → overthinking. After R2 #12 the TUI render count is 2/dispatch, not 3 — agentCompleted no longer calls safeEmitUpdate. R3 #1 added error-arm budgetUpdated to match. |
Test count
- Core: 283 → 287 (+4 R3 tests: fast-path throw, override-path throw, error-arm budgetUpdated, gate-before-count)
- CLI: 47 (unchanged)
- 0 lint, 0 typecheck for workflow-touching files
Branch state
Rebased onto current main (70d6e5f93); 4 commits ahead — P5 + R1 + R2 + R3.
CI lint
The Lint failure is pre-existing main breakage: shellcheck SC2295 in .github/workflows/qwen-autofix.yml:598 introduced by commit a335f9cdf (PR #5233), unrelated to this PR's diff. Leaving alone — should clear once main fixes it.
All 7 round-3 threads resolved.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. — qwen3.7-max via Qwen Code /review
Collaborator
✅ Local runtime verification — budget mechanism works, suites green, types cleanP5 adds a per-run output-token budget to the 1) Test suites — all greenPer the PR's own test plan, in the worktree:
(The PR body's 272/42 are slightly stale — the PR head includes the review-round-3 commits, which added tests; actual counts are higher and all pass.) 2) Independent runtime verification of the real shipped codePure budget functions ( Driving the real
This independently confirms the subtle part — the soft-gate overshoot is real and bounded: a sequential run overshoots a cap of 100 to exactly 120 (check-then-act), matching the module docstring's Notes for the maintainer
Recommendation: the core mechanism is correct, well-tested, type-clean, and backward-compatible ( Verified in a PR-head worktree under 中文版（点击展开）✅ 本地运行验证 —— 预算机制有效、测试全绿、类型干净P5 给 1) 测试套件 —— 全绿按 PR 自己的测试计划，在 worktree 里：
（PR 正文的 272/42 略微过时 —— PR head 包含 review 第 3 轮的提交，新增了测试；实际数量更高且全部通过。） 2) 对真实发布代码的独立运行验证纯预算函数（ 驱动真实的
这独立确认了最微妙的部分 —— 软门 overshoot 是真实且有界的：顺序运行会把上限 100 超到正好 120（check-then-act），与模块 docstring 的 给维护者的提示
建议： 核心机制正确、测试充分、类型干净、向后兼容（ 在 PR-head worktree 下用 |
wenshao
reviewed
Jun 18, 2026
Comment on lines
+692
to
+699
| expect(dispatchCalls).toBe(0); | ||
| // R3 #7 contract: the script saw budget-exceeded errors, NOT | ||
| // agent-count-exceeded errors. The latter would indicate the old | ||
| // ordering still applies. | ||
| // The orchestrator wraps script-thrown errors in | ||
| // WorkflowExecutionError; the script swallowed each per-call throw | ||
| // and returned the last message, so the run COMPLETED successfully. | ||
| expect(caught).toBeUndefined(); |
Collaborator
There was a problem hiding this comment.
[Suggestion] This R3 #7 regression test — added to guard the gate-before-agentCount++ reorder — passes identically whether the fix is present or reverted, so it can't catch a regression of that reorder.
Its only assertions are dispatchCalls === 0 and caught === undefined, and both hold under the old (buggy) ordering too: the in-script try/catch swallows every per-call throw (budget-exceeded or agent-cap), so orchestrator.run(...) always resolves and caught is always undefined. The contract the comment states ("saw a budget error, NOT an agent-cap error") is never asserted — the discriminating value (return lastErr) is thrown away because the resolved result of await orchestrator.run(...) is discarded.
Capture the run outcome and assert the error type (the run returns WorkflowRunOutcome with a .result field, as other tests already read):
// capture the run result (currently discarded) ...
const outcome = await orchestrator.run({ script, args: undefined, budget });
// ... then assert the error TYPE, not merely that the run completed:
expect(dispatchCalls).toBe(0);
expect(String(outcome.result)).toMatch(/exceeded the token budget/);
expect(String(outcome.result)).not.toMatch(/maximum of \d+ agent\(\) calls/);On the old ordering the last error is the agent-cap message, so the toMatch(/exceeded the token budget/) assertion fails — which is exactly the regression signal this test is meant to provide.
— claude-opus-4-8[1m] via Qwen Code /qreview
wenshao
approved these changes
Jun 18, 2026
Collaborator
|
@qwen-code /triage |
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual body included. Direction: Aligned. Workflows can dispatch up to 1000 agents per run — a per-run output-token cap is a real safety need, not a speculative feature. This is phase P5 of the Dynamic Workflows port (#4721), which puts it squarely in the project's roadmap. Upstream Claude Code has workflow token-budget semantics; while the CHANGELOG doesn't name "per-run token cap" explicitly, the area is clearly relevant. Approach: The scope is large (+1892/-14 across 17 files) but justified — the feature genuinely needs to touch core (budget impl + orchestrator gate), registry (tracking + per-phase attribution), tool (banner + wiring), CLI command ( One note: the code comments are unusually dense — nearly every line has a JSDoc rationale. This is helpful for a security-adjacent feature but pushes the diff size. Not a blocker. The R3 fixes (token accounting on 中文说明感谢贡献！ 模板完整 ✓ — 所有必填章节齐全，含双语正文。 方向： 对齐。Workflow 单次 run 可派发多达 1000 个 agent —— 按 run 的 output token 上限是真实的安全需求，不是投机功能。这是 Dynamic Workflows port 的 P5 阶段（#4721），完全在项目路线图中。上游 Claude Code 有 workflow token 预算语义；虽然 CHANGELOG 未明确提及「按 run token 上限」，但该领域显然相关。 方案： 范围较大（17 个文件 +1892/-14），但合理 —— 功能确实需要触及 core（预算实现 + orchestrator 闸门）、registry（追踪 + 按 phase 归属）、tool（横幅 + 接线）、CLI 命令（ R3 修复（ — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal (before reading the diff): to add per-run token budget, I'd create a budget tracker (total/spent/remaining/recordSpent), resolve from env, add a gate in the dispatch path, wire token recording from PR's approach matches and exceeds mine. Three things the PR does better than my baseline:
No new Critical blockers. All prior Critical findings (token accounting on throw path, One minor note: the Test ResultsSuites — ✅ all green
CLI load checkCLI loads cleanly with workflows enabled. Real-LLM E2ECould not replicate — no DashScope / API credentials available in this CI environment. The budget mechanism is thoroughly covered by the 287 unit tests (including R3's throw-path regression tests, parallel-batch overshoot bounds, and the 中文说明代码审查独立方案（看 diff 之前）：加按 run token 预算，我会做一个预算追踪器（total/spent/remaining/recordSpent），从 env 解析，在分发路径加闸门，把 PR 的方案与我的匹配且更优。 三个超越我基线的点：
无新 Critical 阻塞。 之前的 Critical（throw 路径 token 计账、 一个小提醒： 测试结果套件 — ✅ 全绿
真实 LLM E2E无法复现 — CI 环境无 DashScope / API 凭证。预算机制由 287 个单元测试充分覆盖（包括 R3 的 throw 路径回归测试、parallel-batch overshoot 约束测试、 — Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back: this is a well-executed feature PR that does exactly what it says. The budget mechanism is correct (dual gate for parallel safety, try/finally for throw-path accounting), the UI surfacing is thoughtful (banner on first call only, cap chip in dialog, per-phase attribution with a The scope is large but honest — every file in the diff is there for the stated goal. The 287+47 test suites cover the subtle edges (throw path, parallel overshoot, backward deltas, null-sentinel attribution, latch-survives-reset) that would otherwise surface as production bugs. The two prior independent verifications (live tmux E2E + deterministic runtime harness) confirm the mechanism works outside the test sandbox. My one reservation — Clean merge. ✅ 中文说明退一步看：这是一个执行良好的功能 PR，做了它说要做的事。预算机制正确（双重闸门保证 parallel 安全，try/finally 保证抛错路径计账），UI 呈现考虑周到（仅首次调用显示横幅，dialog 中的上限 chip，按 phase 归属 + 范围大但诚实 — diff 中的每个文件都是为了声明的目标。287+47 个测试覆盖了那些否则会在生产中暴露的细微边界（throw 路径、parallel overshoot、backward delta、null-sentinel 归属、latch 在 reset 后存活）。两位之前的独立验证（tmux 真 LLM E2E + 确定性运行时 harness）确认了机制在测试沙箱之外也能工作。 我唯一的保留 — 干净合并 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jun 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
added
category/cli
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Adds a per-run output-token budget to the
Workflowtool. The budget is plumbed through the orchestrator dispatch gate, theWorkflowRunRegistry, theBackgroundTasksDialogphase tree, and the/workflowsslash command. A one-time usage banner is prepended to the first successful workflow result of a session, gated by a newskipWorkflowUsageWarningsetting. Two new knobs:QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<int>(env, per-run cap) andskipWorkflowUsageWarning: true(setting, suppress banner). Implementation includesWorkflowBudgetImpl+ env resolver with a 100M hard ceiling on the override,WorkflowBudgetExceededErrorcarryingrunId / budgetTotal / spent, abudgetUpdatedorchestrator emitter event, registry fieldstokensSpent / tokenBudgetTotal / perPhaseTokenswith anonBudgetUpdatedhandler attributing token deltas tocurrentPhaseat fire time, and ashouldShowUsageWarning()latch that survivesreset()so/cleardoesn't re-arm the banner.Why it's needed
Phase P5 of the Dynamic Workflows port (issue #4721). Workflows can dispatch up to 1000 agents per run; without a per-run output-token cap, a runaway script can burn through significant token budget unchecked. The cap is a soft gate, not a pre-commit reservation — checked at dispatch entry, so concurrent fan-out (
parallel()/pipeline()) inside the concurrency window can overshoot by up to(concurrency_window − 1) × per_dispatch_tokensbefore the first overshooting dispatch throwsWorkflowBudgetExceededError. Matches upstream Claude Code 2.1.168 semantics. The UI surfacing (chip in dialog row + per-phase totals + tokens/cap detail block) lets operators see the budget at glance without needing to readdistlogs; the banner teaches the env knob without spamming the model context (banner lives inreturnDisplayonly, never inllmContent).Reviewer Test Plan
How to verify
Set
QWEN_CODE_ENABLE_WORKFLOWS=1and launchqwen. Ask the model to run a workflow script via theWorkflowtool. Observe (a) the one-time banner in the tool result, (b) the run appearing in/workflows, (c)/workflows <runId>rendering thetokens/cap/ per-phase block. Run a second workflow in the same session and confirm the banner is suppressed. SetQWEN_CODE_MAX_TOKENS_PER_WORKFLOW=100and run a workflow that dispatches multiple agents — confirm the second or third dispatch throwsWorkflowBudgetExceededErroronce cumulative output tokens exceed 100. SetskipWorkflowUsageWarning: truein settings and confirm the banner never appears.Evidence (Before & After)
See the dedicated E2E test report comment (posted separately) for the full real-LLM tmux session capture. Highlights:
Before P5 —
/workflows <runId>detail dump had no tokens / cap / per-phase fields; first Workflow call had no usage banner;QWEN_CODE_MAX_TOKENS_PER_WORKFLOWhad no effect.After P5 (tmux + DashScope
qwen3.7-plus):> Workflows have no per-run token cap. Set 'QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<n>' (env) for a soft cap. Suppress this notice with 'skipWorkflowUsageWarning: true' in settings./workflows:Workflow runs (1 total · 0 running)+ Recent row./workflows wf_xxx:status: completed,runtime: 2ms,agents: 0/0,tokens: 0,cap: (no cap),Phases (1)./skipWorkflowUsageWarning/regex count = 1 across full scrollback).Tested on
Local tmux verification on macOS only; CI runs cover Linux + Windows test suites (typecheck + 272 + 42).
Environment (optional)
Local dev:
node bundle/qwen.jsfromnpm run bundleoutput, OAuthqwen-oauthand--auth-type openai --openai-base-url https://dashscope.aliyuncs.com/compatible-mode/v1 --model qwen3.7-plusboth confirmed working with the new tool registration.Risk & Scope
(concurrency_window − 1) × per_dispatch_tokensbefore the first overshoot dispatch throws. Documented in module-level docstring + banner copy. Matches upstream semantics. Operators sizing the cap should subtract this margin.phase()boundary mid-flight (documented inWorkflowRunRegistry.onBudgetUpdatedJSDoc).WorkflowTaskRegistration.tokenBudgetTotalis optional and defaults tonull; legacy registrations continue working unchanged. ExistingWorkflowOrchestratorEmitterconsumers continue working without settingbudgetUpdated.skipWorkflowUsageWarningdefaults tofalse, banner fires once per session by default.Linked Issues
Refs #4721
中文说明
这个 PR 干了什么
为
Workflow工具增加按 run 的 output token 预算。预算贯通 orchestrator 调度门、WorkflowRunRegistry、BackgroundTasksDialogphase 树，以及/workflowsslash 命令。每个 session 首次成功运行 workflow 时，在结果前 prepend 一次性使用提示横幅，由新设置skipWorkflowUsageWarning控制。两个新旋钮：QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<int>（env，按 run 上限）和skipWorkflowUsageWarning: true（设置，关闭横幅）。实现包含WorkflowBudgetImpl+ env 解析器（env override 上有 100M 硬上限）、WorkflowBudgetExceededError（携带runId / budgetTotal / spent）、budgetUpdatedorchestrator emitter 事件、registry 字段tokensSpent / tokenBudgetTotal / perPhaseTokens+onBudgetUpdatedhandler（按 fire 时刻的currentPhase归属 token delta），以及shouldShowUsageWarning()latch（reset()不重置，避免/clear重新触发横幅）。为什么需要
Dynamic Workflows port 的 P5 阶段（issue #4721）。workflow 单 run 可以派发最多 1000 个 agent；没有按 run 的 output token 上限，跑飞的脚本可以消耗大量 token 不受约束。这个上限是软门，不是预先预留 —— 在调度入口检查，所以并发 fan-out（
parallel()/pipeline()）在 concurrency 窗口内可以 overshoot，最多到(concurrency_window − 1) × per_dispatch_tokens，第一次 overshoot 的调度抛WorkflowBudgetExceededError。与上游 Claude Code 2.1.168 语义对齐。UI 暴露（dialog 行 chip + per-phase 总计 + tokens/cap 详情块）让运维不用读dist日志就能 glance 看到预算；横幅教用户 env 旋钮，但不污染 model context（横幅只在returnDisplay，不进llmContent）。Reviewer 验证计划
如何验证
设
QWEN_CODE_ENABLE_WORKFLOWS=1启动qwen。让 model 通过Workflow工具跑 workflow 脚本。观察 (a) 工具结果里出现一次性横幅，(b) 这次运行出现在/workflows列表里，(c)/workflows <runId>渲染tokens/cap/ per-phase 块。同 session 跑第二次 workflow，确认横幅被压制。设QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=100跑一个派发多个 agent 的 workflow —— 确认累计 output token 超过 100 时第二或第三次调度抛WorkflowBudgetExceededError。设skipWorkflowUsageWarning: true后确认横幅完全不出现。Before / After 证据
详见单独贴出的 E2E 测试报告 comment（含完整 tmux 真场景截屏）。要点：
P5 之前 ——
/workflows <runId>详情没有 tokens / cap / per-phase 字段；首次调用没有横幅；QWEN_CODE_MAX_TOKENS_PER_WORKFLOW不生效。P5 之后（tmux + DashScope
qwen3.7-plus实测）：> Workflows have no per-run token cap. Set 'QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<n>' (env) for a soft cap. Suppress this notice with 'skipWorkflowUsageWarning: true' in settings./workflows：Workflow runs (1 total · 0 running)+ Recent 行/workflows wf_xxx：status: completed、runtime: 2ms、agents: 0/0、tokens: 0、cap: (no cap)、Phases (1)/skipWorkflowUsageWarning/匹配数 = 1）测试平台
macOS 本地 tmux 实测；Linux + Windows 测试套件由 CI 覆盖（typecheck + 272 + 42 个测试）。
环境（可选）
本地 dev：
node bundle/qwen.js（npm run bundle产物），OAuthqwen-oauth和--auth-type openai --openai-base-url https://dashscope.aliyuncs.com/compatible-mode/v1 --model qwen3.7-plus都验证可以触发新工具注册。风险与边界
(concurrency_window − 1) × per_dispatch_tokens。模块级 docstring + 横幅文案都已注明。与上游语义对齐。运维设置上限时应留出这个余量。phase()边界时的归属竞争（WorkflowRunRegistry.onBudgetUpdatedJSDoc 中已说明）。WorkflowTaskRegistration.tokenBudgetTotal是 optional，默认null；旧的 registration 继续无改动工作。已有的WorkflowOrchestratorEmitterconsumer 不实现budgetUpdated也无影响。skipWorkflowUsageWarning默认false，默认每 session 横幅触发一次。关联 issue
Refs #4721