fix(i18n): Correct zh-TW translations to match Traditional Chinese conventions#1
Closed
MikeWang0316tw wants to merge 6 commits into
Closed
fix(i18n): Correct zh-TW translations to match Traditional Chinese conventions#1MikeWang0316tw wants to merge 6 commits into
MikeWang0316tw wants to merge 6 commits into
Conversation
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. |
…nventions Fix ~131 lines of Traditional Chinese (zh-TW) translations that used Simplified Chinese character forms instead of standard Traditional Chinese usage. Changes: - 文件 → 檔案 (47 occurrences) - 爲 → 為 (45 occurrences) - 啓 → 啟 (44 occurrences) - 曆史 → 歷史 (6 occurrences) - 鏈接 → 連結 (4 occurrences) - 菜單 → 選單 (3 occurrences)
MikeWang0316tw
force-pushed
the
feat/fix-zh-tw-translations
branch
from
May 14, 2026 01:37
f17741d to
e31442a
Compare
Align with Traditional Chinese convention where 伺服器 is the standard term for 'server' in computing contexts.
…rite Clarify that the file is the authoritative source and should not be overwritten with auto-generated output, to prevent future maintainers from regenerating with raw OpenCC and losing manual corrections.
Addresses reviewer feedback on PR QwenLM#4129 (points 2 and 3): - scripts/check-i18n.ts: Iterate over parsed zh-TW translation values (not raw file content) and report the offending key. Replace the earlier substring list with ZH_TW_FORBIDDEN_PATTERNS, which targets the three real regression categories: variant Traditional characters produced by OpenCC s2t (爲, 啓), Mainland-Chinese vocabulary (服務器, 菜單, 鏈接), and pure Simplified characters. Excludes 禁用 / 配置 / 文件 / 打開 to avoid false positives on Taiwan-valid usage. - scripts/tests/check-i18n.test.ts: Cover the new check, including negative cases for Taiwan-valid vocabulary. - docs/users/features/language.md: Document zh-TW maintenance — the vocabulary table, why raw OpenCC s2t output is not acceptable, and where the CI-enforced list lives. Co-Authored-By: Claude Opus 4.7 <[email protected]>
- check-i18n.ts: Sort ZH_TW_FORBIDDEN_PATTERNS longest-first and break on first match so e.g. `历史` reports the specific bigram instead of also firing the bare `历` rule (no duplicate CI errors). - check-i18n.ts: Add ZH_TW_ALLOWED_EXCEPTIONS escape hatch so a future legitimate translation (e.g. 區塊鏈 in a UI string) can opt out by key without weakening the global pattern list. - docs/users/features/language.md: Add a "CI enforced?" column so contributors can tell which rows block CI vs. which are review-only style guidance. Replace bare `曆` in the table with the `曆史` bigram and note that `曆` is correct in calendar terms (日曆, 農曆, 西曆) — prevents a future maintainer from globally replacing 曆→歷. - Tests: Cover the dedup behavior on overlapping patterns. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Document the known limitation that `includes()`-based pattern matching does not respect Chinese word boundaries — a bigram like `鏈接` will false-positive on `區塊鏈接口` (區塊鏈 + 接口). Direct contributors to `ZH_TW_ALLOWED_EXCEPTIONS` when this happens instead of weakening the pattern list. Co-Authored-By: Claude Opus 4.7 <[email protected]>
MikeWang0316tw
pushed a commit
that referenced
this pull request
May 26, 2026
…wenLM#4103) (QwenLM#4502) * feat(cli): headless runaway-protection guardrails (QwenLM#4103) Adds two opt-in run-level budgets and a startup safety warning for non-interactive / CI / SDK runs. All defaults preserve existing behavior; the budgets only fire when the user explicitly sets a limit. Phase 1 — surface unsafe configs and fix doc drift - New `--yolo`-without-sandbox stderr warning at startup of every non-interactive run, emitted by `getHeadlessYoloSafetyWarning` in `packages/cli/src/utils/headlessSafetyWarnings.ts`. Suppressible via `QWEN_CODE_SUPPRESS_YOLO_WARNING=1` (strict `1`/`true` match so `=0` / `=false` don't silence it). Strict env match also applied to the `SANDBOX` check so values like `SANDBOX=0` don't accidentally bypass the warning. - Gated on `!config.isInteractive()` at the gemini.tsx call site so TUI users aren't nagged. - `docs/users/configuration/settings.md`: corrected `model.skipLoopDetection` default (`true`, not `false`) and reworded the `--yolo`/sandbox section — `--yolo` does NOT auto-enable a sandbox; sandboxing must still be opted into explicitly. Phase 2 — run-level budgets with distinct exit code - `--max-wall-time` / `model.maxWallTimeSeconds`: wall-clock duration for the whole run. Flag accepts `90` (s), `30s`, `5m`, `1h`, `500ms`. Settings is plain seconds. - `--max-tool-calls` / `model.maxToolCalls`: cumulative tool executions (success + failure). Ticked BEFORE each `executeToolCall` so a budget of N caps the run at exactly N executions. - New `FatalBudgetExceededError` (exit code 55), distinct from `FatalTurnLimitedError` (53) and `FatalCancellationError` (130) so CI scripts can branch on the reason. JSON output mirrors the `handleMaxTurnsExceededError` / `handleCancellationError` envelope convention. - Enforced via `RunBudgetEnforcer` in `packages/cli/src/utils/runBudget.ts`, wired to the same `AbortController` as SIGINT so existing cancellation plumbing carries the abort. A `routeAbort` helper distinguishes budget vs. SIGINT at the abort-check sites and at the outer catch. Critical correctness fixes (informed by the QwenLM#4105 review pass) - Drain-loop fall-through: the inner drain-item `for await` previously exited via `finalizeAssistantMessage(); return;`, swallowing a budget abort that fires during the last drain item and surfacing exit code 0. Now routes through `routeAbort` so exit 55 is preserved. - Settings symmetry: `maxWallTimeSeconds: 0` in settings.json is now rejected (same as `--max-wall-time 0`); the enforcer treats `<=0` as "no timer" so silent disable would be a foot-gun. `validateMaxWallTimeSetting` also rejects `Infinity` / `NaN`. - `setTimeout` overflow: both parser paths reject durations above `Math.floor((2^31 - 1) / 1000)s` (~24.8 days). Node clamps oversized delays to 1ms and fires the timer almost immediately; fail loud at startup instead. - First-fence-wins + SIGINT race: `markExceeded` no-ops if the controller was already aborted by a third party, so a budget tick arriving after user SIGINT doesn't misattribute the abort to exit code 55. - Outer catch re-routes mid-stream `AbortError`s through the budget handler so users see "Run aborted: …" instead of raw "AbortError". Tests - `runBudget.test.ts` (32 tests): parser happy / reject paths, setting validator, post-increment off-by-one, `maxToolCalls=0` meaning "disallowed", `-1` meaning unlimited, wall-clock under fake timers, `stop()` cancels pending timer, idempotent `start()`, first-fence-wins, SIGINT-race protection. - `headlessSafetyWarnings.test.ts` (7 tests): YOLO + sandbox / env matrix; strict-truthy `SANDBOX` check; suppression env. - Pre-existing suites: `nonInteractiveCli.test.ts` (46), `gemini.test.tsx` (23), `config/config.test.ts` (220), `core/utils/errors.test.ts` (12), `core/config/config.test.ts` (172) all green after picking up the new config getters / CliArgs fields. Backward compatibility - All budgets default to `-1` (unlimited); existing CLI invocations behave identically. - New stderr warning only fires in the narrow YOLO-no-sandbox case, with an explicit suppress env. - New exit code 55 is purely additive; no existing exit codes change meaning. * fix(cli): address audit findings for headless guardrails (QwenLM#4103, QwenLM#4502) Round-1 audit (3 angles × line-by-line + removed-behavior + cross-file) plus an open-ended design pass surfaced eight correctness issues. This commit lands all of them; the larger ACP / serve-mode structural items are documented for follow-up. Correctness fixes - headlessSafetyWarnings: `SANDBOX` env check reverted to plain truthy. The sandbox transport sets `SANDBOX` to `sandbox-exec` (macOS seatbelt) or the container name (`qwen-code-sandbox`), neither of which matches `isTruthyEnv`. The PR's strict-`1`/`true` check was emitting the "no sandbox" warning INSIDE real sandboxes. Match the rest of the codebase (sandboxConfig.ts, gemini.tsx, Footer.tsx, prompts.ts, …) which all treat any non-empty value as "sandboxed". - nonInteractiveCli main-loop abort: add `finalizeAssistantMessage()` before `routeAbort()`. The drain-item loop already had it (PR QwenLM#4502 Critical bug #1); the main loop was asymmetric — stream-json consumers would see an unterminated `message_start` when a budget / SIGINT abort landed mid-stream. - nonInteractiveCli drain-loop `routeAbort`: also flush `flushQueuedNotificationsToSdk(localQueue)` and `finalizeOneShotMonitors()` before exiting. The old `return`-and- fall-through path went through the outer holdback loop, which did this flushing; switching to `routeAbort()` skipped it, so `task_started` envelopes lost their paired `task_notification`. - nonInteractiveCli catch handler: emit `adapter.emitResult({...})` BEFORE `handleBudgetExceededError`, with the budget message as `errorMessage` when budget tripped. Previously the budget handler `process.exit(55)`ed before the adapter could emit a terminal `result` envelope, so STREAM_JSON consumers never saw a stream terminator on budget exits and hung waiting for one. - runBudget: new `validateMaxToolCalls` mirrors `validateMaxWallTimeSetting`. yargs coerces non-numeric flag values (`--max-tool-calls abc`) to `NaN`, and the enforcer's `>= 0` gate treats `NaN` and negatives as "no limit", silently disabling the budget. Reject `NaN`, `Infinity`, fractional, and negative-other- than-`-1` values at both flag and settings layers. `0` remains legal (`first tick aborts`), unlike wall-time where 0 is fatal. - runBudget: new `MIN_WALL_TIME_SECONDS = 1` floor. Previously `--max-wall-time 500ms` parsed cleanly and aborted on the next event-loop tick before any model round-trip — almost certainly a typo (`5m`?) and not a useful guardrail at any rate. - nonInteractiveCli `tickToolCall`: exempt `ToolNames.STRUCTURED_OUTPUT`. Under `--json-schema` this is the terminal "I'm done" contract tool, not real work. Without the exemption a budget-edge completion is aborted as a false positive (model used N tools then emitted structured_output as call N+1 → exit 55 instead of success). - commands/serve.ts: emit the YOLO-no-sandbox warning at daemon startup when settings.json statically configures `tools.approvalMode: 'yolo'` with no `tools.sandbox` / `SANDBOX` env. The daemon can't use `getHeadlessYoloSafetyWarning` (no Config yet — sessions get their own) so we re-derive the predicate from settings. Per-session ACP override is documented as out of scope. Documentation - `docs/users/features/headless.md`: new "Scope" subsection under Run-level budgets explaining (a) `--max-tool-calls` counts top-level dispatches only — subagent / `agent` tool inner calls are not counted, (b) `structured_output` is exempt, (c) stream-json input mode resets budgets per user message, (d) `qwen serve` / ACP sessions do not currently consult budgets from settings.json. Tests - `runBudget.test.ts` grows from 32 → 41 tests: `validateMaxToolCalls` (NaN / Infinity / negatives / fractional), `parseDurationSeconds` sub-second rejection, `validateMaxWallTimeSetting` sub-second rejection. - `headlessSafetyWarnings.test.ts`: replaced the "still warns when SANDBOX is 0/false/no" case (which encoded the strict-check bug) with positive coverage for the real sandbox-set values (`sandbox-exec`, `qwen-code-sandbox`). All previously-green suites still green: cli/nonInteractiveCli (46), cli/gemini.test (23), cli/config/config.test (220), core/utils/errors (12), core/config/config.test (172). 337 tests across the touched suites. Won't-fix (out of scope, documented or pre-existing) - Unpaired `tool_use` in stream-json when a tool is aborted mid-execution — pre-existing structural gap (SIGINT mid-tool has the same outcome); PR amplifies it but doesn't introduce it. - Narrow SIGINT-vs-budget-timer race — already mitigated by `markExceeded`'s `signal.aborted` check. - `tickToolCall` increments past abort (cosmetic; only affects the `observed` value in the error envelope for a pathological caller). * fix(cli): round-2 audit fixes for headless guardrails (QwenLM#4103, QwenLM#4502) Round-2 audit (after round-1 commit 40ae6dd) surfaced two NEW correctness issues introduced by the round-1 catch-handler restructure, plus a handful of polish items from a parallel design pass. Correctness fixes (new bugs from R1) - nonInteractiveCli catch handler: wrap `adapter.emitResult` in try/catch. R1 moved the emit BEFORE `handleBudgetExceededError` so STREAM_JSON consumers see a terminal envelope first. But emitResult eventually hits `stdout.write`, which throws on EPIPE / ERR_STREAM_WRITE_AFTER_END when a piped consumer closes early (`qwen -p ... | head -n 1` is the common CI case). Letting that throw bubble out skipped both `handleBudgetExceededError` and `handleError`, dropping the documented exit-code-55 contract precisely when stdout was in trouble. Best-effort emit and continue to the exit handler. - nonInteractiveCli `structured_output` exemption: also require `config.getJsonSchema?.() !== undefined`. Without that guard, an MCP server registering an unrelated tool literally named `structured_output` would silently bypass `--max-tool-calls`. Also documents (in `headless.md` "Scope") the related caveat that failed Ajv-validation retries skip the tick too, so a malformed-output retry loop is NOT bounded by `--max-tool-calls` — combine with `--max-session-turns` or `--max-wall-time`. Polish - runBudget `validateMaxToolCalls` upper bound: cap at 1_000_000. `1e10` (typo for `1e1`) would otherwise parse cleanly, pass the `>= 0` gate forever, and silently disable the budget — the exact foot-gun `MAX_WALL_TIME_SECONDS` was built to prevent. Symmetry. - runBudget `parseDurationSeconds` sub-second hint: only append the "did you mean Ns?" suggestion when the input actually contained `ms`. Bare `0.5` would otherwise produce a useless "did you mean 0.5s?" suggestion. - nonInteractiveCli `routeAbort`: the `throw 'unreachable'` is only hit if `handleBudgetExceededError` / `handleCancellationError` ever becomes resumable (e.g. mocked `process.exit` in a test). Carry the original exceeded.message into the thrown Error so the outer catch's `errorMessage` field stays actionable instead of degrading to a literal "unreachable" string. - commands/serve.ts: compare `approvalMode` against `ApprovalMode.YOLO` enum instead of the string literal `'yolo'`. If the enum value is ever renamed, the startup warning stays in sync with the helper at `headlessSafetyWarnings.ts` instead of silently going dead. Documentation - `headless.md` "Scope": clarify the `structured_output` exemption is unconditional (including failed validations); add explicit note that `--max-session-turns` does NOT exempt `structured_output`, so size to `N+1` for `N` real-work turns under `--json-schema`. - `headless.md` flag table: add `1.5h` to the accepted-forms hint for `--max-wall-time` (the parser already accepts fractional units). Tests - `runBudget.test.ts`: new coverage for the `validateMaxToolCalls` ceiling. Total 42 tests across `runBudget.test.ts` (was 41), all green. cli/nonInteractiveCli, gemini.test, config/config all unchanged and still green. Won't-fix (documented above or out of scope) - ACP per-session approval-mode escalation (mid-session flip to YOLO) doesn't print the warning — daemon-level wiring; out of scope for this PR. - 1s wall-time floor vs higher (5–10s) — debatable, keeping 1s with loud sub-second rejection; can raise later without semver impact. - Integration test for the full budget-trip → catch → emitResult → exit 55 path — requires a process-exit-mocking harness; tracked as follow-up. * docs: align headless guardrails examples with R1 sub-second floor Round-3 audit caught two stale doc surfaces that R1's 1-second wall-time floor (and R2's `1.5h` fractional-unit addition) didn't update: - `docs/users/features/headless.md` budget table: replace stale `500ms` example with `1.5h`, add explicit "minimum 1s — sub-second values are rejected as typos" note. - `docs/users/configuration/settings.md` `model.maxWallTimeSeconds` row: same fix. Also extend `model.maxToolCalls` row with the structured_output exemption note, the `0` semantic, and the 1,000,000 ceiling that R2 added. A user copying the documented `--max-wall-time 500ms` example from either surface would hit a startup error after R1. Known follow-up (not addressed in this commit) - No test exercises the R2 `isStructuredOutputExempt` predicate end-to-end. Adding one needs the same process-exit-mocking harness called out in the R2 commit as a separate follow-up. * docs: align JSDoc / schema / CLI help with R1+R2 validation rules Round-4 final-pass audit caught four schema/help-text/JSDoc surfaces that drifted from the validators introduced in R1 (1s wall-time floor, 24-day ceiling) and R2 (1M tool-call ceiling, structured_output exemption, `0` sentinel). - `runBudget.ts` `parseDurationSeconds` JSDoc: replace stale claim that `500ms` is accepted and "sub-second precision is preserved" with the actual contract — `[MIN_WALL_TIME_SECONDS, MAX_WALL_TIME_SECONDS]`, ms suffix only legal when value resolves to >= 1s. Adds `1.5h` to the accepted-forms list. - `settingsSchema.ts` `model.maxWallTimeSeconds` description: now documents the 1s minimum and ~24-day ceiling. - `settingsSchema.ts` `model.maxToolCalls` description: documents the structured_output exemption, the `0` sentinel ("no tool calls allowed"), and the 1,000,000 ceiling. - `vscode-ide-companion/schemas/settings.schema.json`: mirrors both schema descriptions above so the VS Code settings UI auto-completion matches. - `config.ts` yargs `--max-wall-time` description: documents the 1s floor and the ~24-day max. - `config.ts` yargs `--max-tool-calls` description: documents the structured_output exemption, the `0` sentinel, and the 1M ceiling. `qwen --help` is the most-read surface for these flags; matches the prose docs in headless.md and settings.md. No code changes — pure doc/help-text alignment. --------- Co-authored-by: 克竟 <[email protected]>
MikeWang0316tw
pushed a commit
that referenced
this pull request
Jun 18, 2026
…4721) (QwenLM#5094) * feat(core): Workflow P4a — extractAndStripMeta + meta on RunOutcome (QwenLM#4721) First half of P4 (per the refined QwenLM#4721 plan). Extracts the script's `export const meta = {...}` declaration into a typed object so the workflow tool's display payload, and the future /workflows command + phase-tree UI, can read it without re-parsing the script source. The other half of P4 (slash command + KIND_NAMES extension + phase-tree UI + WorkflowTaskRegistry) is queued as a follow-up PR. Architecture: reuse the P1 brace-walker (zero-dep, no parser deps) to locate the meta object literal's source range, then evaluate the literal inside a fresh `vm.createContext(Object.create(null))` — null-prototyped globalThis, no host bridge (no `args` / `process` / `require` / workflow- sandbox globals). The vm realm still exposes its OWN intrinsics (`Object` / `Math` / `Date` / `JSON`), which is fine: meta extraction is one-shot at tool invocation, not replayed on resume. validateMeta walks the eval result field-by-field and copies into a fresh host-realm plain object — no JSON round-trip needed because every contract field is a primitive. User-visible additions: - `extractAndStripMeta(source)` exported from workflow-sandbox.ts - `WorkflowMeta` interface (`{ name, description, whenToUse?, phases?: Array<{title, detail?, model?}> }`) — verbatim shape from upstream Claude Code 2.1.168 - `WorkflowSandbox.getMeta()` accessor alongside `getPhases()` / `getLogs()` - `WorkflowRunOutcome.meta: WorkflowMeta | null` (non-breaking add) - `WorkflowExecutionError.meta: WorkflowMeta | null` so the failure display shows the workflow's name / description / phases even when the script body throws - `WorkflowTool.execute` adds `meta` to the returnDisplay payload when present (omitted when the script had no meta) Error messages verbatim from upstream where applicable: - `meta.name must be a non-empty string` - `meta.description must be a non-empty string` Refactor: P1's `stripExportMeta` is preserved as a thin wrapper around a new `findMetaBlockBounds` helper that both old and new functions share. All 86 existing sandbox tests pass unchanged (no behavior regression in the strip path). Tests: - 11 new `extractAndStripMeta` unit tests covering happy path, optional fields, missing-required validation, malformed shape, vm-eval failure, null-prototype globalThis (no `args` / `process` / `require`), and unbalanced braces - 3 new `createWorkflowSandbox.getMeta()` integration tests - 3 new `WorkflowOrchestrator` outcome.meta tests (null path, parsed path, meta-survives-body-throw on the error path) - 3 new `WorkflowTool` display payload tests (meta in payload, omitted when absent, present on failure path) Suite: 207/207 workflow + adjacent regression green; typecheck + lint clean on packages/core. (Pre-existing acp test type errors in packages/cli are unrelated; CI will confirm.) Related QwenLM#4721 (parent design — multi-phase, not closed by this PR) Related QwenLM#4732 (P1) QwenLM#4947 (P2) QwenLM#5034 (P3) — all merged P4b follow-up: /workflows command + TaskKind workflow union + BackgroundTasksPill KIND_NAMES + phase-tree UI + WorkflowTaskRegistry * test(core): close P4a adversarial-review gaps + add real-LLM E2E (QwenLM#4721) After PR QwenLM#5094 opened without an E2E run, ran a 3-lens adversarial review (correctness / security / completeness) of the meta-extraction assertion strength against extractAndStripMeta and the meta-on-outcome threading path. All 3 reviewers refuted the claim that the existing assertions catch realistic regressions. Triage: - 18 of 24 findings already covered by workflow-sandbox.test.ts (string-with-brace, comments-inside-meta, phases[].model, missing-description error text, args/process/require unreachability, Promise/Math.constructor escape, etc.) - 4 findings (regex literal / template literal / `/m` flag / spread) are host-side parse-path branches the brace walker handles structurally but without explicit negative tests - 2 truly novel gaps closed here: 1. HIGH × 3 lenses: a regression in validateMeta that returns the vm-realm `raw` value directly (skipping the host-realm copy at workflow-sandbox.ts:283-294) would re-open T1/T8/T14 realm escape via outcome.meta.constructor.constructor('return process')(). Vitest toEqual is structural and does NOT check prototype identity, so every prior assertion in the suite would still pass. Add returned-meta + phases array + phase entries prototype- identity check in workflow-sandbox.test.ts; mirror end-to-end in the live test's scenario A. 2. MEDIUM: meta-shaped result collision — if a script returns `{ name, description, phases }`, the safeStringifyDisplayPayload spread must keep `meta` and `result` distinct at the top level. Add a workflow.test.ts case that returns a meta-shaped object and asserts both display.meta and display.result hold their own distinct values. Also add the real-LLM E2E harness at workflow-p4a-meta-live.live.test.ts (6 scenarios: meta+agent, no-meta, malformed-meta short-circuit, body-throw with meta preservation, parallel() fan-out with meta phases, pipeline() multi-stage with meta phases). The suite is gated by DASHSCOPE_API_KEY — describe.skip when absent, so CI without the env shows 0 tests in this file rather than failing. Verified locally 6/6 against qwen3-coder-plus via DashScope OpenAI-compatible endpoint. Final test count: 129/129 (89 sandbox + 34 tool + 6 live). * fix(core): P4a meta-literal Promise crash + live-test typecheck + prettier (QwenLM#4721) Round 3 review fixes: 1. **(Critical, wenshao R1)** A Promise — typically from `import('node:fs')` inside a meta literal — used to crash the host process. `runInContext` evaluates the literal synchronously and returns; `validateMeta` drops the non-contract field silently; the workflow returns its result; THEN the dangling unhandled rejection terminates the process under Node's default `--unhandled-rejections=throw`, decoupled from the run that triggered it. Wenshao reproduced on Node 22.22 with: export const meta = { name:'x', description:'d', extra: import('node:fs') } return 1 → run returns 1, process exits with code 1. Mitigation: after `vm.Script(...).runInContext(...)`, walk the eval result recursively, call `.catch(() => {})` on any thenable to mark the rejection handled, and throw an explicit "meta values must not be Promises" so the malformed meta is rejected before validation continues. Recursion covers `phases[]` entries embedding `import()` below the top level. Two RED-first regression tests in workflow-sandbox.test.ts (top-level + nested-in-phases). 2. **(Critical, wenshao R2/R3)** `tsc --noEmit` failed with 11 errors in the new live E2E test file, blocking CI Lint + all 3 Test jobs: - TS2459 (L36): `WorkflowAgentOpts` is exported from `workflow-sandbox.js`, not from `workflow-orchestrator.js` — fixed import path. - TS2322 (L108/165/220): typing `liveDispatch` as `WorkflowAgentDispatch` widens the return to `string | object`, which doesn't fit `lastText: string`. Dropped the type annotation; the inferred `Promise<string>` is still assignment-compatible with `WorkflowAgentDispatch` (string ⊂ string | object). - TS2345 (×6): `WorkflowRunRequest.args` is required (`args: unknown`, not optional). Added `args: undefined` to every `orch.run({ script })` call. 3. Prettier: `--write` on the 4 touched files. R2 also flagged this; pre-commit lint-staged would normally cover it but the live test file's TS errors short-circuited it. Final local verification: - `tsc --noEmit`: 0 errors - 209/209 tests pass across workflow-sandbox + workflow-orchestrator + workflow.test.ts + live (6 scenarios against qwen3-coder-plus via DashScope) * feat(core+cli): Workflow P4b — /workflows command + phase-tree UI + WorkflowRunRegistry (QwenLM#4721) P4b completes phase P4 of the Dynamic Workflows port. P4a (already on this branch, commits 5b56c39 / 55c23a0 / 402df8f) locked the meta contract: outcome.meta / err.meta / display payload. P4b adds the consumer side — visible workflow runs in the TUI. ## Core (4 changes, 1 new file) - `TaskKind` widened in `packages/core/src/agents/tasks/types.ts` from 3 → 4 variants (adds `'workflow'`). `TaskState` union picks up `WorkflowTask` automatically. - New `WorkflowRunRegistry` (`packages/core/src/agents/workflow-run- registry.ts`) — sibling of `BackgroundTaskRegistry` / `BackgroundShellRegistry` / `MonitorRegistry`. Same register / cancel / get / list / on('statusChange') shape; per-kind state holds runId, meta, current phase, phase history, dispatch counters, recent logs. Eviction: `MAX_RETAINED_TERMINAL_WORKFLOWS = 10` mirrors monitor cap. - `Config.getWorkflowRunRegistry()` exposed via the same Object.create override pattern as the other registries. - `WorkflowOrchestratorEmitter` interface added to workflow-sandbox.ts — fires `phaseStarted` (from sandbox safePhase), `agentDispatched` / `agentCompleted` (from orchestrator countedDispatch), and `logAppended` (from sandbox safeLog). Defensive try/catch around every emit so a subscriber error never bubbles into the script. Orchestrator accepts optional `runId` in WorkflowRunRequest so callers can pre-generate the id and register the run BEFORE run() resolves. - `WorkflowTool` now registers the run with the registry at execute() start, wires the emitter to the registry's update methods + the tool's _updateOutput callback, flips `canUpdateOutput` to `true` for live phase-tree rendering, and routes terminals to registry.complete / fail / cancel (cancel on signal.aborted so user intent stays distinct from script bugs). - `WorkflowRunRegistry` exported from core/index.ts. ## CLI (5 changes, 2 new files) - `BackgroundTasksPill.tsx` `KIND_NAMES` gains `workflow: { singular, plural }`. Counts accumulator + sort order updated: `shell → agent → monitor → workflow → dream` (user-initiated before system-initiated). - `useBackgroundTaskView.ts` subscribes to the workflow registry alongside the existing three; `entryId` switch adds `case 'workflow': return entry.runId`; cleanup unsubscribes. - `BackgroundTasksDialog.tsx` adds `WorkflowDetailBody` (inline, matches MonitorDetailBody style) — renders workflow name, description, status, runtime, current phase, agent dispatch counts (M/N), the phase tree (capped at MAX_VISIBLE_PHASES=20 with "+N more above"), and the log tail (capped at MAX_VISIBLE_LOG_LINES=10). rowLabel switch surfaces `[workflow] <name> · <phase> (M/N)`. DetailBody + statusVerb switches gain workflow cases. - `BackgroundTaskViewContext.tsx` cancelSelected: `case 'workflow': registry.cancel(runId, Date.now())`. Idempotent with the WorkflowTool's signal.aborted catch path. - `BackgroundTasksDialog.test.tsx` entryId mock gains workflow case. ## New /workflows slash command - `packages/cli/src/ui/commands/workflowsCommand.ts` + tests. Bare `/workflows` lists active + completed runs (running first, then terminal by endTime DESC). `/workflows <runId>` opens a per-run detail dump (meta block, status, runtime, phase tree, recent logs, errors). - Gated by `Config.isWorkflowsEnabled()` in BuiltinCommandLoader — command vanishes from typeahead when the flag is off. Already- defined env-var overrides (`QWEN_CODE_ENABLE_WORKFLOWS` opt-in, `QWEN_CODE_DISABLE_WORKFLOWS` kill switch) inherited for free. - Interactive mode adds a "Tip: focus the Background tasks pill" redirect; non-interactive / acp modes omit the tip since they have no dialog. ## Scope deferrals - **ACP daemon protocol widening** (acp-bridge bridgeTypes / status / tasksSnapshot) is deferred to a follow-up PR. Workflows remain invisible to SDK + web-shell consumers in P4b; the CLI-internal surface is complete. - **Phase-tree token rollup** (per-phase token totals in the detail body) needs P5's budget tracker. The infrastructure (registry records, emitter fire sites) is ready for the column when P5 lands. - **Save / inspect subcommands** (`/workflows save <runId>` to materialize a script) are future enhancements; the slash command ships with list + detail only. ## Verification - 223/223 tests pass on the workflow surface — 14 new registry tests, 6 real-LLM E2E scenarios against qwen3-coder-plus (DashScope), plus all existing P3/P4a tests continuing to pass with the new emitter wiring. - 73/73 CLI tests pass across BackgroundTasksPill, BackgroundTasks Dialog, useBackgroundTaskView, workflowsCommand. - `tsc --noEmit` clean (0 P4b errors in core + cli). - `prettier --check` + `eslint` clean on all touched files. * fix(core): bound rejectThenablesInMeta against cyclic meta input (QwenLM#4721) Round 4 review fix. **(Suggestion, wenshao R4)** The R3 thenable walker recursed without a cycle guard. A meta literal that builds a cyclic object via spread overflows the call stack: export const meta = { name: 'x', description: 'y', ...(function () { const a = {}; a.self = a; return a; })(), } vm-eval returns the cyclic object cleanly; `rejectThenablesInMeta` walks `Object.values(...)` and recurses into `a.self === a` forever, producing `RangeError: Maximum call stack size exceeded`. The walker exists to reject Promises before they leave a dangling rejection, but the walk itself must terminate on any shape vm-eval can return — not just on the happy-path acyclic shape. Fix: thread an optional `seen = new WeakSet<object>()` parameter, early-return on `seen.has(value)`. Bounds the recursion against both cycles AND shared subgraphs (where the same node is reached through multiple keys), and keeps the walker O(N) on the eval'd size. Two RED-first regression tests: - Direct self-reference via spread: `{ ...{self: itself} }` - Cycle reached through nested arrays/objects: `{ ...{items: [{ref: outer}]} }` Both previously threw RangeError; both now succeed (validateMeta silently drops the non-contract `self` / `items` fields, so the returned meta is just `{ name, description }` — only reachable if the walker terminates first). `validateMeta` does NOT recurse into nested objects (it walks the top-level contract fields + iterates `phases[]` one level deep with direct property access), so no sibling drift — only the thenable walker needed the guard. * test(cli): stub isWorkflowsEnabled in BuiltinCommandLoader mock config (QwenLM#4721) CI fix for c3f9d84 (Workflow P4b). P4b added a gated `workflowsCommand` to BuiltinCommandLoader: this.config?.isWorkflowsEnabled() ? workflowsCommand : null, The optional-chain only guards `config` being null/undefined — once config is truthy, `.isWorkflowsEnabled()` invokes the method directly. The existing `mockConfig` in BuiltinCommandLoader.test.ts stubbed `isLspEnabled` / `getFolderTrust` / `getManagedAutoMemoryEnabled` but never `isWorkflowsEnabled`, so the new call hit `undefined()` and threw `TypeError: this.config?.isWorkflowsEnabled is not a function`. 10 tests (×3 OS) red on `e9ad07683` for this single reason. Add `isWorkflowsEnabled: vi.fn().mockReturnValue(false)` to the mock, matching the existing pattern. Default to `false` so the loader does not add `workflowsCommand` to the assertions that count exact builtin output — those tests are unchanged. * test(core): cover P4b registry integration + orchestrator emitter (QwenLM#4721) Round 5 fix for the two Critical findings on the P4b commit. ## workflow.test.ts — registry integration seam (+3 tests) \`fakeConfig()\` returns \`{}\`, so \`config.getWorkflowRunRegistry?.()\` short-circuits to undefined in every existing test. The whole P4b integration path inside \`WorkflowTool.execute()\` — \`register()\` on start, the emitter closure firing into the registry, post-run \`complete()\`, catch-arm \`fail()\` / \`cancel()\` branching — is never exercised. Add a \`configWithRegistry()\` helper that builds a config holding a real \`WorkflowRunRegistry\` and returns the registry handle for inspection. Three new tests pin: - **success path**: registry entry transitions to \`completed\` with meta synthesised from \`meta.name\` (the tool fast-tracks description = meta.name when default = runId), correct phases array, agent counts \`1/1\`, script result mirrored, \`endTime\` set. - **failure path**: registry entry transitions to \`failed\`, error message recorded verbatim, phases up to the throw preserved. - **abort path**: pre-aborted signal causes the catch arm to record \`cancelled\` (not \`failed\`) so the dialog distinguishes user-initiated stops from script bugs. ## workflow-orchestrator.test.ts — emitter callbacks (+3 tests) The \`emitter\` field on \`WorkflowRunRequest\` and its firing sites (sandbox \`safePhase\` / \`safeLog\`, orchestrator \`countedDispatch\` before + after) are the only channel keeping the registry record in sync with the live run. Three new tests pin: - **happy-path ordering**: with all four callbacks wired to an event log, the script \`phase('Plan') → log('starting') → agent → phase('Build') → agent\` emits seven events in expected order with expected payloads (\`label\` threaded through both \`agentDispatched\` and \`agentCompleted\`). - **rejection path**: dispatch throwing \`dispatch-boom\` fires \`agentCompleted(label, 'dispatch-boom')\` — pins the symmetric emit-on-throw at workflow-orchestrator.ts:1124 catch arm. - **defensive try/catch**: every callback throwing should be swallowed so orchestration still completes. Pins each emit site's try/catch wrapper individually. Total: 6 new tests, all green. typecheck 0, prettier clean. * fix(core+cli): R7 review fixes — 6 substantive + tmux re-verified (QwenLM#4721) Wenshao's R7 review (with real build + tmux verification on the merged state) approved the PR but surfaced 6 valid findings. All fixed, all RED-first tested, all verified end-to-end against qwen3-coder-plus via DashScope. ## 1. Dialog-cancel drops accumulated logs (Critical) `registry.setRecentLogs(...)` previously guarded `status === 'running'` only. Dialog-initiated cancel marks `status='cancelled'` synchronously BEFORE the tool's catch arm tries to write logs — so cancelled runs always showed an empty Logs section in the dialog. Allow the write after a `'cancelled'` transition too; keep `'completed'` and `'failed'` as final-state rejects. 2 regression tests: cancel-then-logs-still- writes, and complete/fail-still-reject. ## 2. WorkflowRunRegistry missing session-reset wiring (Critical) Sibling drift miss from P4b: `BackgroundTaskRegistry` / `BackgroundShellRegistry` / `MonitorRegistry` all exposed `reset()` + `abortAll()`, and `backgroundWorkUtils` (`hasBlockingBackgroundWork`, `resetBackgroundStateForSessionSwitch`) wired all three. `Workflow RunRegistry` had neither. Result: `/clear` and session-resume ran while a workflow was mid-run (orphaned dispatch loop), and terminal rows leaked from session to session in the pill / dialog / `/workflows` list. Added `hasRunningEntries()`, `reset()` (drops entries, no controller touch), `abortAll()` (cancels every running entry + aborts its controller). Wired into both `backgroundWorkUtils` helpers + updated their tests for the 4-sibling shape. ## 3. Phase dedup inconsistency — sandbox vs registry (Critical) `phase('X'); phase('X')` previously yielded `outcome.phases = ['X','X']` (sandbox `safePhase` unconditional push) but `entry.phases = ['X']` (registry `onPhaseStarted` collapsed). The same run showed different phase counts in the terminal `returnDisplay` JSON vs the live UI. The `agent({phase})` wrapper already deduped (`__b.lastPhase()`); my docstring on `safePhase` claimed it deduped too, but it didn't. Fix at the sandbox layer (single source of truth): `safePhase` skips when `phases[last] === t`. Registry-side dedup is now redundant but harmless (defense in depth, doesn't double-collapse). Updated test in workflow-sandbox.test.ts pins `phase('X'); phase('X'); phase('Y'); phase('X')` → `['X','Y','X']`. ## 4. /workflows tip pointed at non-functional path (UX/docs) `/workflows` tip text said *"focus the Background tasks pill in the footer (use ↓ from an empty composer) and press Enter for the interactive dialog with phase tree + live updates."* But `setPillFocused(true)` doesn't exist anywhere in the codebase (confirmed by wenshao's grep, and reproducible on my own tmux runs where `↓ Enter` never opened the dialog — I previously misattributed to a tmux limitation). The dialog IS reachable through other paths but the tip's specific instructions are wrong. Soften the tip to point at the actually-working text-mode detail view: `Tip: use /workflows <runId> for the per-run detail view (name, description, phase tree, recent logs).` — same information, working instructions. ## 5. `runId` validation comment was aspirational (docs) Comment on `workflow-orchestrator.ts` `run()` claimed *"validates the shape (`wf_<hex>`)"* but the code is `const runId = req.runId ?? generateRunId();` — no validation. Caller (`WorkflowTool`) does use the same `wf_<8hex>` generator as `generateRunId()`, so the behavior is safe in practice. Fixed the comment to describe what the code actually does (trusts the caller, no validation). ## 6. Duplicate extractAndStripMeta test (test hygiene) Two tests at workflow-sandbox.test.ts:227 and :242 used identical source `{ name: args.x, description: 'd' }` — copilot R1 originally flagged this, I declined as bot finding, wenshao re-confirmed. The intent was to pin two distinct things: (a) generic unknown identifier throws, (b) the bridge global `args` specifically is not reachable. Updated the first test to use `totallyUnknown` (a genuine unknown name) and kept the second as the explicit `args` regression — now the two tests pin different things. ## Verification - 231/231 core workflow tests pass (registry + sandbox + orchestrator + tool) — +6 new from R7 RED-first regression tests - 81/81 CLI ripple tests pass (pill + dialog + hook + command + backgroundWorkUtils) - tsc 0 errors on core + cli (after rebuilding core dist for the new registry methods) - prettier + eslint clean on all touched files - **tmux re-verified end-to-end** against qwen3-coder-plus via DashScope on a fresh `npm run bundle`: - Phase dedup: `phase("Phase A"); phase("Phase A")` → `outcome.phases = ["Phase A", "Phase B"]` (was `["Phase A", "Phase A", "Phase B"]` pre-fix) — confirmed both in the tool result JSON AND in `/workflows wf_xxx · 2 phases` - New `/workflows` tip text rendered as expected, no more advertising broken pill focus path - `/workflows <runId>` detail dump still works: name, status, runtime, phases tree, agent counts * test(core): R7 dialog-cancel race integration test (QwenLM#4721) The unit test in workflow-run-registry.test.ts pins the setRecentLogs guard widening in isolation. This integration test stands up the full production wiring — real WorkflowTool, real WorkflowRunRegistry, real sandbox, real emitter — and reproduces the exact dialog-cancel race that the R7 fix targets: 1. Start execute() with a controllable dispatch that hangs until externally rejected. 2. Wait for the run to register + the dispatch to be in flight + at least one log() call to have accumulated. 3. Simulate the dialog: call registry.cancel(runId) directly. This is the exact entry point cancelSelected() uses in BackgroundTaskViewContext.cancelSelected for kind='workflow'. 4. Cascade the dispatch rejection (the production path: the registry's abortController abort propagates through dispatchController → the orchestrator's limiter → the in-flight dispatch). 5. Await execute() — the tool's catch arm runs setRecentLogs with the accumulated logs. 6. Assert: status='cancelled' AND recentLogs contains the script's log('before agent dispatch') entry. RED→GREEN verified: temporarily reverted the setRecentLogs guard to the pre-R7 single-state form, the test failed with `AssertionError: expected 0 to be greater than 0` (recentLogs was empty because the guard rejected). Restored fix, test passes. Production reachability note: the dialog itself is not currently reachable through the TUI pill focus chain (setPillFocused(true) does not exist anywhere in the codebase — wenshao R7 verification finding #1, pre-existing infra gap out of P4 scope). This integration test is the closest available real-scenario verification for the fix without modifying out-of-scope code; the test drives the EXACT registry.cancel + dispatch rejection + catch-arm sequence the dialog would trigger. * test(cli): stub getWorkflowRunRegistry in clearCommand + useResumeCommand mocks (QwenLM#4721) CI fix for b53bc4e (the R7 fix commit). R7 (commit b53bc4e) wired WorkflowRunRegistry's new reset() / abortAll() / hasRunningEntries() methods into backgroundWorkUtils.ts (hasBlockingBackgroundWork + resetBackgroundStateForSessionSwitch). clearCommand.ts and useResumeCommand.ts call both utils, but their mock configs didn't stub getWorkflowRunRegistry — so once the util started calling it, every clearCommand test threw `TypeError: config.getWorkflowRunRegistry is not a function`. CI ubuntu/macos/windows × 10 tests × clearCommand.test.ts went red. This is the same sibling-drift miss as the BuiltinCommandLoader fix in 2491911 (R7 pre-cursor): I added a new Config method, every existing mock that ships a Config-shaped object goes stale until stubbed. Fix: add the same `getWorkflowRunRegistry` stub shape to: - clearCommand.test.ts: 3 sites (default mock + non-interactive mock + blocked-background mock) - useResumeCommand.test.ts: 4 sites (all 4 mock configs) Stub shape mirrors the 3 sibling registries' interfaces exposed via backgroundWorkUtils: `hasRunningEntries`, `reset`, `abortAll`. Returns false / vi.fn() so default behavior matches "no workflow running" + "reset is observable". Existing tests that exercise the blocked-background path (hasUnfinalizedTasks: true) keep working because workflow's hasRunningEntries=false still allows the agent-side block to trigger. Verification: 24/24 in clearCommand+useResumeCommand pass locally; the 7-file P4 + impacted suite pass 105/105 (registries + commands + hooks + dialog + pill + workflowsCommand + backgroundWorkUtils).
MikeWang0316tw
pushed a commit
that referenced
this pull request
Jun 19, 2026
QwenLM#5231) * feat(core,cli): workflow tool token budget + per-run UI surfacing (P5) P5 of the Dynamic Workflows port (QwenLM#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 QwenLM#4721. * fix(core,cli): close P5 review round 1 — token tracking gaps + UI polish (PR QwenLM#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). - QwenLM#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. - QwenLM#4 (workflow-orchestrator.ts): added debugLogger.warn at both gate sites (entry + intra-limiter) for budget-rejected dispatches. Suggestion fixes: - QwenLM#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. - QwenLM#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. - QwenLM#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. - QwenLM#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. - QwenLM#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 QwenLM#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 QwenLM#8 emit gating; rewrote the backwards/zero-delta test to the new monotonic-spent contract. - workflow.test.ts: +1 R1 test for QwenLM#10 (capped banner shape — was untested; only the uncapped shape had coverage). - workflowsCommand.test.ts: +1 R1 test for QwenLM#6 null-sentinel surfacing. Updated assertions for QwenLM#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). QwenLM#9 fold: the parallel-batch overshoot test serves as the regression guard for the intra-limiter gate fix in #2. QwenLM#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: QwenLM#5231 * fix(core,cli): close P5 review round 2 — UI emit dedup + error tail + dialog coverage (PR QwenLM#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: - QwenLM#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. - QwenLM#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. - QwenLM#15 (BackgroundTasksDialog.test.tsx): WorkflowDetailBody had no rendering test coverage. Added 4 cases under a new R2 QwenLM#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: - QwenLM#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 QwenLM#14 negative assertion), 42 → 47 CLI (+5 R2 QwenLM#15 + null-sentinel coverage). 0 lint, 0 typecheck for workflow-touching files. PR: QwenLM#5231 * fix(core): close P5 review round 3 — finally-bracket execute(), error-arm budgetUpdated, gate-before-count (PR QwenLM#5231) 3 fixes for round 3 review. wenshao (human maintainer) caught a real production-path token leak that R1's `R1 QwenLM#3` test missed; bot also found that R2 QwenLM#12 (UI emit dedup) left the error arm with zero re-renders. Critical fixes (orchestrator core): - QwenLM#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 QwenLM#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 QwenLM#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 QwenLM#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. - QwenLM#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 QwenLM#3 (debugLogger on NaN drop in recordSpent): hostile-provider defensive hardening, R5 → overthinking. - bot QwenLM#4 (debugLogger on negative delta in onBudgetUpdated): same. - bot QwenLM#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 QwenLM#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: QwenLM#5231
MikeWang0316tw
added a commit
that referenced
this pull request
Jun 19, 2026
Regression coverage for the state-leak fixed in 04fcffd (doudouOUC Critical #1/#2, confirmed by wenshao's maintainer re-verification): Tab, Right-arrow and Enter accepts plus message submit must each call onPromptSuggestionDismiss, so the persisted promptSuggestion can't reappear as a ghost placeholder when the buffer is next cleared. Co-Authored-By: Claude Opus 4.8 <[email protected]>
MikeWang0316tw
pushed a commit
that referenced
this pull request
Jul 7, 2026
… model persistence (QwenLM#6060) * feat(cli): add --project and --global flags to /model for per-project model persistence Add scope control to the /model command so users can persist model selections to either project-level or user-level settings independently. - /model --project: persist to workspace .qwen/settings.json - /model --global: persist to user ~/.qwen/settings.json - /model (no flag): unchanged behavior (backward compatible) - Model dialog title shows scope: 'Select Model (this project)' / 'Select Model (global)' - Completion and argumentHint updated with new flags - Full i18n support for zh/en Closes QwenLM#6052 Signed-off-by: Alex <[email protected]> * fix(cli): add missing zh-TW translations for /model scope flags Signed-off-by: Alex <[email protected]> * fix(cli): address PR review — scope flags, subcommand persistScope, titles, tests - parseScopeFlags: use (?:^|\s) instead of \b for --flag matching (\b fails because - is not a word character) - Completion: strip all flags to isolate model prefix, supports any order - Subcommand dialogs (fast/voice/vision) now propagate persistScope - slashCommandProcessor forwards persistScope for all subcommand cases - ModelDialog title combines subcommand mode + scope label e.g. 'Select Fast Model (this project)' - Subcommand confirmations show scope suffix (project/global) - Extract persistScopeSpread() helper to reduce duplication - Add 9 tests covering scope flags, dialog returns, confirmations - Add i18n keys for scope suffix labels in zh/en/zh-TW Signed-off-by: Alex <[email protected]> * fix(cli): use Partial<Config> & {[key:string]:unknown} to fix index signature TS error Replace Record<string,unknown> with Partial<Config> & {[key:string]:unknown} to satisfy TS4111 index signature access rule in the CI build. Signed-off-by: Alex <[email protected]> * fix(cli): add scope suffix to ModelDialog history items Address review comment: historyManager.addItem for voice/fast/vision/main model selections now shows scope indicator like ' (this project)' or ' (global)', consistent with CLI direct-set confirmations. Affected: handleModelSwitchSuccess (main), handleSelect (voice/fast/vision) Signed-off-by: Alex <[email protected]> * fix(cli): wrap scopeSuffix in t() and unify wording with ModelDialog - scopeSuffix in modelCommand.ts now uses t(' (this project)') / t(' (global)') instead of hardcoded English strings, matching ModelDialog.tsx wording - Main model confirmation uses shared scopeSuffix instead of separate i18n keys, eliminating 'Model: {{model}} (project)' duplication - Remove unused i18n keys from en/zh/zh-TW locales - Update tests to expect '(this project)' wording Signed-off-by: Alex <[email protected]> * fix(cli): address code review feedback — scope validation, i18n, tests - Reject inline prompt + scope flag combination with clear error (#1) - Add mutual exclusivity check for --project and --global (QwenLM#5) - Verify setValue scope parameter in tests + add --global test (#2) - Extract scopeSuffix to shared variable, remove duplication (QwenLM#3) - Remove dead i18n keys 'Select Model (this project)' / '(global)' (QwenLM#4) - Fix scopeSuffix placement on model line not API key line (QwenLM#8) - Add fr.js / ja.js translations for scope keys (QwenLM#10) - Remove unused export ModelDialogPersistScope (QwenLM#6) - Wrap non-interactive help text in t() with new flags (QwenLM#7) - Fix argumentHint grouping to show mode vs scope flags (QwenLM#11) Signed-off-by: Alex <[email protected]> * fix(cli): reject --project when workspace is untrusted Reject --project scope flag before direct persistence or opening ModelDialog when settings.isTrusted is false. Workspace settings are ignored on merge in that state, so the save would silently not take effect. Also mirrors the guard in ModelDialog.tsx resolvePersistScope() to fall back to user scope when the dialog is opened with --project on an untrusted folder. Default mock settings now includes isTrusted: true. Signed-off-by: Alex <[email protected]> --------- Signed-off-by: Alex <[email protected]> Co-authored-by: qwen-code-dev-bot <[email protected]>
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
1 participant
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.
This PR fixes ~131 lines of Traditional Chinese (zh-TW) translations in
packages/cli/src/i18n/locales/zh-TW.jsthat used Simplified Chinese character forms instead of standard Traditional Chinese usage.Changes
The following categories of corrections were applied:
文件→檔案爲→為啓→啟曆史→歷史鏈接→連結菜單→選單Rationale
These corrections align the zh-TW locale with standard Traditional Chinese usage:
Testing
npm run devto verify UI displays correctly with updated translations.