Skip to content

fix(autofix): clamp gate test load explicitly instead of via RUNNER_NAME - #10671

Merged
yiliang114 merged 6 commits into
QwenLM:mainfrom
qqqys:fix/autofix-gate-load-clamps
Sep 1, 2026
Merged

fix(autofix): clamp gate test load explicitly instead of via RUNNER_NAME#10671
yiliang114 merged 6 commits into
QwenLM:mainfrom
qqqys:fix/autofix-gate-load-clamps

Conversation

@qqqys

@qqqys qqqys commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Passes the ECS load-clamp values explicitly on the verification gate's two vitest invocations (the per-package --changed origin/main run and the bite check), via a shared VITEST_LOAD_CLAMPS array: --maxWorkers=25% --testTimeout=60000 --hookTimeout=60000 --coverage.enabled=false. The values are the repository's own ECS calibration from packages/*/vitest.config.ts; coverage is turned off because nothing in the gate or the report path consumes it.

Why it's needed

The gate launches through an env -i allowlist (qwen-autofix.yml:5431/5691) that — deliberately, to drop $GITHUB_ENV plants — passes CI, HOME, PATH etc. but not RUNNER_NAME. The vitest configs' ECS clamps key on RUNNER_NAME starting with ecs-qwen-, so inside the gate they silently deactivate: 15s test timeouts, unbounded workers (64 cores per job), coverage collection on, on hosts shared with other autofix jobs (max-parallel: 20).

Under pool saturation this one gap produced both dominant gate failure classes of the last week:

Passing the flags explicitly keeps the verdict independent of env plumbing and runner naming, and leaves the fortified env -i launch chain (structurally pinned by qwen-autofix-workflow.test.js) untouched. The issue-fix verify step shares this script, so both gates are covered by one edit.

Reviewer Test Plan

  1. npx vitest run scripts/tests/qwen-autofix-workflow.test.js --config ./scripts/tests/vitest.config.ts — 229/229 pass (the suite's behavioral runs execute the modified invocation lines through the npm stub; the strip_runner_channels npm run test pin is unchanged).
  2. bash -n .github/scripts/run-autofix-review-verification.sh — clean.
  3. Flag acceptance on the repo's vitest 3.2: npx vitest run <file> --maxWorkers=25% --testTimeout=60000 --hookTimeout=60000 --coverage.enabled=false parses and runs (all three named flags are in vitest run --help; the dotted coverage override is standard vitest CLI config syntax).
  4. After merge, on the next takeover round: the gate log's vitest output should show clamped workers, and a saturated-host round should reject/pass on real signal instead of dying at the 60-minute wall.

Tested on

OS Status
🐧 Linux

Risk & Scope

  • Main risk: --testTimeout=60000 also applies where a workspace config sets a lower deliberate timeout — CLI flags override config for every gate test run. That is the same trade the ECS lanes already accept for regular CI, and only inside the gate; the PR's own CI checks are unaffected.
  • Coverage off is gate-only; regular CI coverage lanes are untouched.
  • Not addressed here (follow-ups per the takeover incident review): unverified update-branch pushes, infra-vs-real failure accounting in the brake, preserving discarded round work.

Linked Issues

Incident evidence: #10171 (takeover rounds 1–7), #10543 (five consecutive gate errors).

中文说明

这个 PR 做了什么

在验证门的两个 vitest 调用点(按 package 的 --changed origin/main 运行和 bite 检查)通过共享的 VITEST_LOAD_CLAMPS 数组显式传入降载参数:--maxWorkers=25% --testTimeout=60000 --hookTimeout=60000 --coverage.enabled=false。取值就是仓库 packages/*/vitest.config.ts 里既有的 ECS 校准值;coverage 关闭是因为 gate 和报告路径都不消费它。

为什么需要

gate 经 env -i 白名单启动(qwen-autofix.yml:5431/5691),白名单(有意地,为了丢弃 $GITHUB_ENV 注入)传了 CIHOMEPATH 等,但没传 RUNNER_NAME。而 vitest 配置的 ECS 降载钳制以 RUNNER_NAME 前缀 ecs-qwen- 为开关,于是在 gate 内静默失效:15 秒测试超时、worker 不限量(每 job 吃满 64 核)、coverage 全开,而主机同时承载多个 autofix job(max-parallel: 20)。

池饱和时,这一个缺口同时制造了上周两类主要 gate 故障:

显式传参让 verdict 不再依赖环境变量管道和 runner 命名,同时完全不动被 qwen-autofix-workflow.test.js 结构性钉死的 env -i 启动链。issue-fix 验证步骤复用同一脚本,一处修改覆盖两个 gate。

风险与范围

  • 主要风险:--testTimeout=60000 会覆盖工作区配置里更低的刻意超时——但这与 ECS 泳道在常规 CI 已接受的取舍相同,且仅限 gate 内;PR 自身的 CI 检查不受影响。
  • coverage 关闭仅限 gate;常规 CI 的 coverage 泳道不动。
  • 本 PR 不处理(接管事故复盘的后续项):update-branch 裸推、熔断的 infra/真实失败记账、被丢弃轮次的工作保存。

🤖 Generated with Claude Code

https://claude.ai/code/session_01AhZA7LdQXZjcjfiPsoZkqZ

The verification gate launches through an env -i allowlist that drops
RUNNER_NAME, so the vitest configs' ECS load clamps (60s test/hook
timeouts, maxWorkers 25%) silently deactivate inside the gate: tests run
with 15s timeouts, unbounded workers and coverage collection on a host
shared with other autofix jobs. Under pool saturation this produced both
false rejections (QwenLM#10171 round 3: 73 load-induced 15s timeouts in files
the PR never touched, charged to the round) and gate deaths past the
step's 60-minute cap that discarded verified fixes ("verification-gate
error": QwenLM#10171 rounds 1/2/5-7, QwenLM#10543 five in a row).

Pass the clamp values explicitly on both gate vitest invocations (the
per-package --changed run and the bite check) so the verdict does not
depend on env plumbing or runner naming, and disable coverage: nothing
in the gate consumes it, and its collection dominated the overrun
(72,000 CPU-seconds of collect in one 1,560s gate leg).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AhZA7LdQXZjcjfiPsoZkqZ
@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 041d8c4 did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 041d8c4 有 1 个 PR CI workflow 未以绿色完成,详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required section is present in substance.

Problem: observed, not theoretical. The failure modes are documented with concrete incidents, and I verified them: run 33245675108 ("Qwen Autofix", 2026-08-29) did conclude as failure, and #10171 (open, takeover rounds in flight) and #10543 (merged after five consecutive gate errors) are real PRs. The root cause also checks out in code: both gate launch sites in qwen-autofix.yml start this script through an env -i allowlist that passes PATH/HOME/CI/… but deliberately not RUNNER_NAME, while the vitest configs in packages/{core,cli,acp-bridge} key their ECS clamps on RUNNER_NAME starting with ecs-qwen- — so inside the gate the clamps genuinely deactivate (15s test timeouts, unbounded workers, coverage on).

Direction: aligned — this is reliability of the repo's own autofix verification gate, and making the verdict independent of env plumbing and runner naming is the right goal. Doesn't touch auth/sandbox/model selection/telemetry/release or any public contract.

Size: not applicable — .github/scripts/ is not a core module path; 23 production lines in one file.

Approach: minimal and sound. One shared VITEST_LOAD_CLAMPS array applied to the script's two vitest invocations (the per-package --changed run and bite_runner_default) — those are the only two test invocations in the script, and both gate launch sites share this script, so one edit covers both gates. Explicit flags are the better of the two natural fixes: the alternative (adding RUNNER_NAME to the allowlist) would keep the gate coupled to runner naming and widen an allowlist the design deliberately keeps small. The declared trade-off (CLI flags overriding workspace-config timeouts) is smaller than stated: no workspace config sets a sub-60s timeout in its ECS branch, so on the pool where the gate runs the clamps are exactly the calibration the ECS lanes already use.

Risk: no elevated risk signals — none of the high-risk paths are touched.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到,非理论性。失败模式有具体事故佐证,且已逐一核实:run 33245675108("Qwen Autofix",2026-08-29)确以 failure 结束;#10171(仍开放,接管轮次进行中)与 #10543(连续五轮门错误后合并)均真实存在。根因亦经代码确认:qwen-autofix.yml 两处门启动点均通过 env -i 白名单启动本脚本,白名单传 PATH/HOME/CI 等但(有意地)不传 RUNNER_NAME,而 packages/{core,cli,acp-bridge} 的 vitest 配置的 ECS 钳制以 RUNNER_NAME 前缀 ecs-qwen- 为开关——因此门内钳制确实静默失效(15 秒测试超时、不限 worker、coverage 全开)。

方向:对齐——这是仓库自身 autofix 验证门的可靠性修复,让 verdict 脱离环境变量管道与 runner 命名是正确目标。不涉及 auth/沙箱/模型选择/遥测/发布或任何公共契约。

规模:不适用——.github/scripts/ 非核心模块路径;单文件 23 行生产代码。

方案:最小且合理。一个共享 VITEST_LOAD_CLAMPS 数组应用于脚本仅有的两处 vitest 调用(按包 --changed 运行与 bite_runner_default);两处门启动点共享同一脚本,一处修改覆盖两个门。显式传参是两种自然修法中更好的一个:替代方案(把 RUNNER_NAME 加入白名单)会让门继续耦合 runner 命名,并扩大设计上刻意精简的白名单。已声明的取舍(CLI 标志覆盖工作区配置超时)实际更小:ECS 分支下没有配置设置低于 60 秒的超时,门运行的池上钳取值正是 ECS 泳道已使用的校准值。

风险:无升级风险信号——未触及任何高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code review

My independent take before reading the diff: "ECS clamps silently deactivate because RUNNER_NAME doesn't survive the env -i allowlist" has two natural fixes — widen the allowlist, or pass the clamp values explicitly at the invocation sites. The PR picks the latter, and I agree it's the better one: the allowlist is deliberately minimal, and explicit flags make the verdict independent of runner naming entirely. The values are the repository's own ECS calibration, not new numbers.

The implementation matches that proposal one-to-one, and I verified the load-bearing claims against the code:

  • Both launch sites in qwen-autofix.yml start this script through an env -i allowlist that omits RUNNER_NAME — so the clamps in packages/{core,cli,acp-bridge}/vitest.config.ts (60s test/hook timeouts, maxWorkers: '25%', keyed on the ecs-qwen- prefix) genuinely fall back to 15s timeouts and unbounded workers inside the gate.
  • The script has exactly two vitest invocations — the per-package --changed origin/main run and bite_runner_default — and both now receive the clamps. Nothing else in the script runs tests, and both gate launch sites share this script, so one edit covers both.
  • The structural pins in scripts/tests/qwen-autofix-workflow.test.js survive: the strip_runner_channels npm run test assertion is a prefix match the diff preserves, and the env -i launch-chain pins target the workflow file, which this PR doesn't touch.
  • The flags are valid on vitest 3.2.4 (--maxWorkers accepts percent strings, --testTimeout/--hookTimeout are documented options, dotted --coverage.enabled=false is standard CLI config-override syntax); array expansion is properly quoted. Per triage rules I did not execute the script or re-run the suite here — that signal comes from the PR's own CI below.
  • The declared trade-off is smaller than stated: no workspace config sets a sub-60s timeout in its ECS branch, so --testTimeout=60000 never lowers a deliberate ceiling on the pool where the gate runs. Turning coverage off costs nothing either — nothing in the gate or the report path consumes vitest coverage output (the script's "coverage" mentions are its own test-line-count advisory).

No critical findings. The change is minimal, every line earns its place, and the comment block documents the incident history where future readers will look for it.

Test evidence

The PR's own CI is still running on the reviewed commit — no failures so far. The workflow-test suite that pins this script runs inside Test (ubuntu-latest); the finalize job rewrites the table once CI settles.

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

Check Conclusion
Test (ubuntu-latest, Node 22.x) ❌ failure
web-shell E2E Smoke (ubuntu-latest, Node 22.x) 🚫 cancelled
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Secret scan (TruffleHog) ✅ success

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

Sandboxed verification would settle the one runtime unknown: @qwen-code /verify — that the gate's two vitest invocations actually accept the clamp flags end-to-end is not provable from the diff or from the suite (which pins the script's text through the npm stub, not the real vitest CLI), and the saturation behavior itself is only observable in a real takeover round after merge.

中文说明

代码审查:独立推导下此问题有两条自然修法——扩 env -i 白名单,或在调用点显式传钳制值;本 PR 选后者,我也认为更优(白名单刻意精简,显式传参使 verdict 彻底脱离 runner 命名),取值即仓库自身的 ECS 校准。实现与独立方案一一对应,关键断言均已对照代码核实:两处门启动点的 env -i 白名单确实不传 RUNNER_NAME,三个包 vitest 配置中的钳制(60 秒测试/钩子超时、maxWorkers: '25%')确实因此在门内回退为 15 秒超时与不限 worker;脚本仅有两处 vitest 调用(按包 --changed 运行与 bite_runner_default),均已加上钳制,两个门共享同一脚本;qwen-autofix-workflow.test.js 的结构性钉不受影响(strip_runner_channels npm run test 为前缀匹配,env -i 启动链的钉针对未被修改的 workflow 文件);四个标志在 vitest 3.2.4 下均有效,数组展开正确加引号。按审查规则未执行脚本、未代跑套件,测试信号来自下方 PR 自身 CI。已声明的取舍实际更小:ECS 分支下没有配置设置低于 60 秒的超时,--testTimeout=60000 不会压低门运行池上的任何刻意上限;关闭 coverage 亦无代价——门与报告路径均不消费 coverage 输出(脚本中的 "coverage" 提及均为其自身的测试行数审计)。无 Critical 发现;改动最小、注释到位。

测试证据:该提交的 PR 自身 CI 仍在运行,暂无失败;钉住本脚本的工作流测试套件包含在 Test (ubuntu-latest) 中,CI 结束后 finalize 任务会更新表格。唯一运行时未知项(vitest CLI 是否端到端接受这些标志)可由 @qwen-code /verify 沙箱验证;饱和场景下的实际行为只能在合并后的真实接管轮次观察。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean, minimal fix for a well-evidenced problem; the only reservation is that the saturation behavior itself can only be confirmed by real gate rounds after merge.

Honest read: this is the shape of fix the gate should wave through. The problem is real and documented — two incident PRs, a failed run I verified, quoted logs — not a hypothesis. The root cause checks out in code rather than being taken on faith, and the diff does exactly one thing: applies the repository's own ECS calibration to the two vitest invocations that were silently losing it. No scope creep, no drive-by edits, and explicit flags over widening the env -i allowlist is the more robust of the two natural approaches. The comment block preserves the incident history right where future readers will look for it. Six months from now this reads as a boring, correct reliability fix — which is the compliment.

The one thing this PR's CI cannot prove is the central claim itself: that under pool saturation the gate rejects on real signal instead of dying at the 60-minute wall or charging load-induced timeouts to the round. That is only observable in production (the author's test plan item 4 says the same). The mechanism is direct enough — the flags are the exact documented clamps, already proven through the config path on the ECS lanes — that I'm comfortable approving on mechanism plus suite, with /verify available to pin the last runtime unknown if a maintainer wants it.

CI is still running on the reviewed commit (unit suite and no-AK integration in progress, no failures so far), so approval is deferred until CI lands green on 041d8c40d03f6ddfc39fbeccb201c098136d766e.

中文说明

置信度 4/5:针对证据充分的问题的干净、最小修复;唯一保留意见是饱和场景下的实际行为只能在合并后的真实门轮次中确认。问题真实且有据(两个事故 PR、一次已核实的失败 run、日志引用),根因经代码确认而非轻信,diff 只做一件事:把仓库自身的 ECS 校准显式应用到两处静默失效的 vitest 调用。无范围蔓延、无顺手改动;显式传参优于扩 env -i 白名单。CI 无法证明的是核心主张本身——池饱和时门以真实信号拒绝、而非撞 60 分钟墙或把负载超时记到轮次账上——这只能在生产环境观察;机制足够直接(标志即已文档化的钳制,且已在 ECS 泳道经配置路径验证),可以基于机制加套件批准,如需钉住最后一个运行时未知项可用 @qwen-code /verify。该提交的 CI 仍在运行(单元套件与 no-AK 集成进行中,暂无失败),故批准延后至 CI 全绿。

Qwen Code · qwen3.8-max

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed.

Not reviewed: reverse audit — stopped before round 1 by the review time budget.

中文说明

仅完成部分审查,审查缺口已披露。

未审查:the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed。

未审查:反向审计——评审时间预算不足,未能开始第 1 轮。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment on lines +1020 to +1022
VITEST_LOAD_CLAMPS=(
--maxWorkers=25%
--testTimeout=60000

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R1-1: [fails-closed] [regression] --maxWorkers=25% crashes the vitest 1.6.1 that packages/sdk-typescript pins, so every gate leg for that workspace exits 1 with zero tests collected and the round is rejected deterministically.

The shared clamp array is passed to every workspace's vitest, but packages/sdk-typescript pins vitest ^1.6.0 (the lockfile nests 1.6.1 there while the root hoists 3.2.7). Any autofix round whose changed files resolve to packages/sdk-typescript runs its per-package leg into the crash below, and run_check_no_ab then charges tests failed in packages/sdk-typescript to the round regardless of the fix's correctness — re-introducing, deterministically, the exact false-rejection class this PR exists to eliminate. On the bite path the same crash is masked (any failure reads as "biting"), silently voiding the check's all-green power for sdk rounds.

Witness:

vitest 1.6.1 (lockfile-pinned sdk-typescript copy) + exact VITEST_LOAD_CLAMPS flags:
PR arm:  vitest run --maxWorkers=25% --testTimeout=60000 --hookTimeout=60000 --coverage.enabled=false
  -> RangeError: Invalid array length, Test Files no tests, EXIT=1
BASE arm (no flags): Test Files 1 passed (1), EXIT=0
flip arm (three flags without --maxWorkers): Test Files 1 passed (1), EXIT=0
vitest 3.2.7 with the same four flags -> Test Files 1 passed (1), EXIT=0

Suggested fix: don't pass --maxWorkers unconditionally — derive the worker flag per workspace from its pinned vitest major (append --maxWorkers=25% only for vitest >= 2, keeping the other three flags, which 1.6.1 accepts). A simple flag swap does not work: vitest 3.2.7 rejects --maxThreads (CACError: Unknown option '--maxThreads'), so no single worker-clamp spelling is valid across both majors.

The fix must keep the cli/core legs on the repo's exact ECS calibration — packages/core/vitest.config.ts:17-26: 60_000 / 60_000 / '25%' keyed on RUNNER_NAME starting ecs-qwen- — and the structural pin toContain('strip_runner_channels npm run test') at scripts/tests/qwen-autofix-workflow.test.js:8945 must keep matching any rewritten bite invocation. Please add a workflow-test case asserting every workspace the gate can drive accepts every flag in VITEST_LOAD_CLAMPS under its pinned vitest major — restoring the unconditional --maxWorkers while sdk-typescript pins vitest 1.x must turn it red.

中文说明

R1-1:--maxWorkers=25% 会让 packages/sdk-typescript 锁定的 vitest 1.6.1 崩溃,该工作区的每个 gate 测试腿都会零收集直接 exit 1,轮次被确定性拒绝。

共享降载数组会传给每个工作区的 vitest,但 packages/sdk-typescript 锁定 vitest ^1.6.0(lockfile 在该包下嵌套 1.6.1,根目录提升 3.2.7)。任何改动文件解析到 packages/sdk-typescript 的 autofix 轮次,其按包测试腿都会触发上面的崩溃,run_check_no_ab 随后把 tests failed in packages/sdk-typescript 记到轮次账上,与修复本身是否正确无关——确定性地重新引入了本 PR 要消除的那类假拒绝。bite 路径上同样的崩溃会被掩盖(任何失败都读作 "biting"),静默废掉 sdk 轮次的全绿判定力。

(证据见英文区 Witness 代码块:1.6.1 加四个参数崩溃、去掉 --maxWorkers 后通过、3.2.7 加同样四个参数通过。)

建议修复:不要无条件传 --maxWorkers——按工作区锁定的 vitest 主版本派生 worker 参数(仅对 vitest >= 2 追加 --maxWorkers=25%,其余三个参数保留,1.6.1 可接受)。简单换参数名不可行:vitest 3.2.7 拒绝 --maxThreads(CACError: Unknown option '--maxThreads'),不存在两个主版本都接受的单一 worker 降载写法。

修复必须保持 cli/core 腿使用仓库现有的 ECS 校准——packages/core/vitest.config.ts:17-26:60_000 / 60_000 / '25%',以 RUNNER_NAME 前缀 ecs-qwen- 为开关——且 scripts/tests/qwen-autofix-workflow.test.js:8945 的结构钉 toContain('strip_runner_channels npm run test') 必须在任何重写后的 bite 调用上继续匹配。请补充一个 workflow 测试,断言 gate 能驱动的每个工作区在其锁定 vitest 主版本下都接受 VITEST_LOAD_CLAMPS 的每个参数——在 sdk-typescript 仍锁定 vitest 1.x 时恢复无条件 --maxWorkers 必须让该测试变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declined — the crash as stated does not reproduce on this branch, and the residual risk the finding correctly points at is now pinned by this round's R2-2 fix.

Evidence: round 2 ran the lockfile-pinned vitest 1.6.1 suite under packages/sdk-typescript with the full clamp array (37 files, 1747 tests, exit 0), and R2-2's own three-arm probe confirms the shape: with sdk's poolOptions.threads.maxThreads: 4 shield present the flags pass, without --maxWorkers they pass, only the shield-less arm crashes. The leg does not exit 1 today, so the deterministic-rejection premise does not hold at this head.

The round-2 decline rationale was nevertheless wrong, and this reply corrects it: --maxWorkers is not "the spelling both majors accept" in any meaningful sense — vitest 1.6.1 accepts the flag NAME but coerces the percent with Number('25%') -> NaN, and survives only because its pool builder reads a numeric poolOptions.threads.maxThreads before ctx.config.maxWorkers. The suggested per-major worker-flag derivation remains a valid future option, but the shield is what keeps the current shape alive, so this round pins THAT instead of trusting it: the new shield case in scripts/tests/unit-vitest-configs.test.ts asserts every vitest-1.x workspace the gate can drive keeps a numeric threads maxThreads (and no pool: 'forks'), and goes red when maxThreads is removed from packages/sdk-typescript/vitest.config.ts (mutation-verified). The cli/core legs stay on the repo's exact ECS calibration and the strip_runner_channels npm run test pin is untouched. Thread left open for maintainer visibility of the corrected rationale.

中文说明

拒绝(Declined)——按原文陈述的崩溃在本分支上无法复现;该发现正确指出的残余风险已由本轮 R2-2 修复钉住。

证据:第 2 轮曾用完整降载数组在 packages/sdk-typescript 锁定的 vitest 1.6.1 下跑完整个套件(37 文件、1747 用例、exit 0),R2-2 自己的三臂探针也证实同一形态:有 sdk 的 poolOptions.threads.maxThreads: 4 屏障时参数通过,去掉 --maxWorkers 时通过,只有无屏障的一臂崩溃。该测试腿今天不会 exit 1,因此"确定性拒绝"的前提在当前 head 上不成立。

但第 2 轮拒绝时给出的理由是错的,本回复予以更正:--maxWorkers 并不是任何有意义层面的"两个主版本都接受的写法"——vitest 1.6.1 接受该参数名,却会把百分值用 Number('25%') 强转成 NaN,其之所以幸存,仅因池构建器先读数值型 poolOptions.threads.maxThreads 再读 ctx.config.maxWorkers。按主版本派生 worker 参数的建议仍是合理的未来选项,但当前形态靠的是这个屏障,因此本轮选择钉住屏障而不是依赖它:scripts/tests/unit-vitest-configs.test.ts 新增的屏障用例会断言 gate 能驱动的每个 vitest-1.x 工作区都保留数值型 threads maxThreads(且不是 pool: 'forks'),并在从 packages/sdk-typescript/vitest.config.ts 移除 maxThreads 时变红(已变异验证)。cli/core 腿保持仓库现有的 ECS 校准,strip_runner_channels npm run test 结构钉不受影响。线程保持打开,以便维护者看到更正后的理由。

Comment thread .github/scripts/run-autofix-review-verification.sh Outdated
Comment thread .github/scripts/run-autofix-review-verification.sh Outdated
Comment thread .github/scripts/run-autofix-review-verification.sh
Comment thread .github/scripts/run-autofix-review-verification.sh Outdated
@yiliang114
yiliang114 enabled auto-merge September 1, 2026 07:16
yiliang114
yiliang114 previously approved these changes Sep 1, 2026
Addresses review round 1 on QwenLM#10671.

R1-6 The contracts check runs a web-shell vitest from inside the gate's
own `env -i` child, and web-shell's config sets no timeouts at all, so
that leg ran at vitest's 5s default on the same saturating host — the
false-rejection class this PR removes, surviving in a sibling path. The
clamp array moves above the contracts call and is handed to the shared
script through AUTOFIX_VITEST_FLAGS; the issue-fix gate invokes the same
script where RUNNER_NAME is present and leaves the variable unset, so its
invocation is unchanged.

R1-3 Nothing pinned that the clamps reach any invocation — every existing
assertion is a prefix that ends before the expansion, so dropping it from
a leg (or emptying the array, silent without `set -u`) stayed green while
the gate reverted to 15s timeouts and coverage on. Three structural pins
added on the review runner only, plus a contracts-script case that runs
with AUTOFIX_VITEST_FLAGS set and asserts the flags reach npm.

R1-4 The array hand-copies the ECS branch of three vitest configs, and
inside the gate the CLI flags outrank the config — so raising an ECS
ceiling to shelter a heavier test would leave the gate enforcing the old
one and rejecting a fix that is green in normal CI. A parity test in
scripts/tests/unit-vitest-configs.test.ts re-imports core, cli and
acp-bridge under a stubbed ecs-qwen RUNNER_NAME (they read the env at
import time) and asserts equality with the array parsed out of the shell
script.

R1-5 Narrowed the comment's claim, per the finding's own minimum. The
residual is real and now named in the script: a handful of test files set
their ceiling with a runtime `vi.setConfig` keyed on RUNNER_NAME, which
outranks the CLI, so they keep their non-ECS values in here. Closing it
needs a gate sentinel on both env -i allowlists plus a change in each
file — a separate slice, not folded into this one.

R1-1 is declined; see the thread. Its two premises did not reproduce
against the lockfile-pinned vitest 1.6.1 under packages/sdk-typescript:
the full suite passes with --maxWorkers=25% (37 files, 1747 tests,
exit 0), and --maxThreads is rejected by 1.6.1 and 3.2.7 alike, so
--maxWorkers is the spelling both majors accept rather than neither.
@qqqys qqqys added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Sep 1, 2026

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

Not reviewed: the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed.

Not reviewed: the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed.

Not explored to full depth (tool budget reached): "agent 1a": could not execute the sdk-typescript suite under --testTimeout=60000 to settle whether the (60s, 180s] band is populated (no node_modules in the review worktr…; "agent 1b": full run of scripts/tests/qwen-autofix-workflow.test.js to confirm no pre-existing assertion breaks against the inserted AUTOFIX_VITEST_FLAGS lines (static ….

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

未审查:the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed。

未审查:the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed。

未探索到全部深度(达到工具调用预算):"agent 1a"could not execute the sdk-typescript suite under --testTimeout=60000 to settle whether the (60s, 180s] band is populated (no node_modules in the review worktr…"agent 1b"full run of scripts/tests/qwen-autofix-workflow.test.js to confirm no pre-existing assertion breaks against the inserted AUTOFIX_VITEST_FLAGS lines (static …

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff,除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上;并会一直如此,直到某一轮的标记重新带上锚点,或落地的嫁接能被运行该轮的评审使用。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment thread .github/scripts/check-autofix-contracts.sh Outdated
Comment thread .github/scripts/run-autofix-review-verification.sh
Comment thread .github/scripts/run-autofix-review-verification.sh
Comment thread scripts/tests/qwen-autofix-workflow.test.js Outdated
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 2 finishedview run. See this round's report below.

中文说明

AutoFix 第 2 轮已完成 —— 查看运行。本轮报告见下方。

…ecord

Addresses review round 2 on QwenLM#10671; re-verifies round 1's fixes with
mutation probes.

R2-1 The comments justifying the unclamped issue-fix leg rested on a
premise that does not hold for that leg: web-shell's vitest config sets
no timeouts and has no RUNNER_NAME branch, so the drift test runs at
vitest's 5s default wherever it runs. Corrected at all four mirror
sites (both scripts and both test comments): the review gate passes
explicit clamps; the issue-fix gate and repo-hygiene's docker leg —
the previously unnamed third caller — invoke the contracts script
without the variable and accept the 5s default. The alternative
(exporting AUTOFIX_VITEST_FLAGS in the issue-fix gate step) edits a
workflow file this PR has never touched and stays out of scope.

R2-2 --maxWorkers=25% is coerced to NaN by vitest 1.x; the
lockfile-pinned 1.6.1 under packages/sdk-typescript survives only
because its config sets a numeric poolOptions.threads.maxThreads,
which tinypool reads before ctx.config.maxWorkers. Pin the shield:
a new case derives vitest-1.x workspaces from nested lockfile copies
and asserts each keeps the threads pool and a numeric maxThreads,
failing with a directive if such a workspace is missing from the
config registry. Mutation-verified red on shield removal.

R2-3 The export is the only line carrying the clamps across the
process boundary into check-autofix-contracts.sh; nothing pinned it.
Added the structural pin plus an ordering assertion against the
contracts call — deleting the export or moving it below the call now
fails the suite. Both mutants verified red.

R2-4 The contracts case's fake npm logged $*-joined argv, rendering a
joined-blob flag byte-identically to separate words; the [*]-for-[@]
mutant survived. The shim now logs one bracketed line per argv word
and the four expectations in the case were updated; the mutant now
fails.

R1-3/R1-4/R1-5/R1-6 (round-2 commit) re-verified with mutation
probes: dropping either invocation's expansion, dropping the
assignment, emptying the array, drifting --testTimeout to 61000, and
dropping the flag expansion inside the contracts script each turn an
existing witness red.

R1-1 remains declined: the deterministic crash does not reproduce at
this head (the leg passes with sdk's shield present), but the round-2
rationale was wrong and is corrected on the thread; the residual risk
is the shield R2-2 now pins.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round 3 — PR #10671

Nine inline findings triaged: four implemented this round, four re-verified as
already fixed by the earlier commit 8f96efe3c0 (mutation probes re-run), one
declined with its record corrected.

Implemented this round

[R2-2] Pin the vitest-1.x maxThreads shield instead of trusting it.
New case in scripts/tests/unit-vitest-configs.test.ts
(autofix gate load clamps): it derives the vitest-1.x workspaces from the
lockfile's nested copies (restricted to workspace dirs; the hoisted-copy >= 2
premise is itself pinned), and asserts each one keeps the threads pool and a
numeric poolOptions.threads.maxThreads — the value tinypool reads before
ctx.config.maxWorkers, which is what keeps --maxWorkers=25% (coerced to
Number('25%') = NaN by vitest 1.x) from crashing the leg. A vitest-1.x
workspace missing from the config registry fails with a directive message.
Mutation-verified: removing maxThreads: 4 from
packages/sdk-typescript/vitest.config.ts turns the test red.

[R2-3] Pin the export AUTOFIX_VITEST_FLAGS line. Added
toContain('export AUTOFIX_VITEST_FLAGS') next to the existing assignment
pin, plus an ordering assertion that it sits above the
bash "${RUNNER_TEMP}/check-autofix-contracts.sh" call — run_check_no_ab
spawns a child bash that inherits exported variables only, so a missing or
late export silently leaves the drift leg at vitest's 5s default.
Mutation-verified both ways: deleting the export line and moving it below the
contracts call each turn the pin red.

[R2-4] Make the fake npm shim preserve argv boundaries. The shim in the
contracts functional case now logs one bracketed line per argv word
(printf '[%s]\n' "$@") instead of a $* join, and all four expectations in
the same it were updated to the per-word form. Mutation-verified: the
"${vitest_flags[@]}""${vitest_flags[*]}" mutant (flags forwarded as one
joined argv word) renders a single element containing a space and fails the
expectation.

[R2-1] Correct the unclamped-issue-fix-leg comments to what is actually
true
(the finding's second, explicitly-offered option). Four sites updated:
check-autofix-contracts.sh, run-autofix-review-verification.sh (the
contracts-call comment), and the two mirror test comments. They now state that
web-shell's config sets no timeouts and has no RUNNER_NAME branch, so the
drift test runs at vitest's 5s default wherever it runs; the review gate
passes its clamps through AUTOFIX_VITEST_FLAGS; the issue-fix gate and
repo-hygiene's docker leg (the previously unnamed third caller) invoke the
script without the variable and accept that default. The alternative first
option — exporting AUTOFIX_VITEST_FLAGS in the issue-fix gate step — would
edit .github/workflows/qwen-autofix.yml, a file this PR has never touched
(the review gate and issue-fix gate copies would then need coordinated pins
across an area outside this PR's footprint); the comment correction resolves
the finding's factual complaint without that expansion.

Re-verified (fixed by 8f96efe3c0, probes re-run this round)

[R1-3] Clamps-reach-invocation pins: dropping the expansion from the
per-package leg, from the bite runner, dropping the AUTOFIX_VITEST_FLAGS
assignment, and emptying the array each turn a witness red (probes P5, P6,
P7, P10).

[R1-4] Clamp/config parity test: drifting the shell --testTimeout to
61000 turns it red (probe P8).

[R1-5] The gate-script comment is the narrowed claim
("at the vitest-config layer") with the named residual population documented;
verified present. Closing the residual itself (gate sentinel on both env -i
allowlists + per-file changes) remains the separate slice round 2 recorded.

[R1-6] Third-leg clamping: dropping the AUTOFIX_VITEST_FLAGS assignment
or the flag expansion inside check-autofix-contracts.sh turns the pins and
the functional case red (probes P7, P9).

Declined

[R1-1] The deterministic-crash claim does not hold at this head — round 2
ran the lockfile-pinned vitest 1.6.1 sdk suite under the full clamp array
(37 files, 1747 tests, exit 0), and R2-2's own three-arm witness shows the
leg passes with sdk's maxThreads shield present. The round-2 rationale was
nevertheless wrong ("the spelling both majors accept" — 1.6.1 accepts the flag
name but coerces the percent to NaN); the reply on the thread corrects that
record, and the residual risk is now pinned by the R2-2 shield test instead
of the larger per-major worker-flag derivation. Thread left open with the
corrected rationale for maintainer visibility.

Mutation probes (all reverted after each probe)

# Mutant Witness Result
P1 remove maxThreads: 4 from sdk vitest config shield test red
P2 delete export AUTOFIX_VITEST_FLAGS export pin red
P3 move export below the contracts call ordering assertion red
P4 ${vitest_flags[@]}${vitest_flags[*]} (joined blob) contracts case red
P5 drop clamp expansion from per-package leg pin 1 red
P6 drop clamp expansion from bite runner pin 2 red
P7 drop AUTOFIX_VITEST_FLAGS assignment pin 3 red
P8 shell --testTimeout=6000061000 parity test red
P9 drop flag expansion inside check-autofix-contracts.sh contracts case red
P10 empty VITEST_LOAD_CLAMPS array parity test red

git status after all probes shows only the four intended files modified;
packages/sdk-typescript/vitest.config.ts was touched only transiently by P1.

Conflict notes

--conflict false; no merge performed.

Verification

  • npm run build — passed
  • npm run typecheck — passed (run again after the final shield-test refinement)
  • npm run lint — passed (run again after the final shield-test refinement)
  • vitest scripts/tests/unit-vitest-configs.test.ts — 25 passed (25)
  • vitest scripts/tests/qwen-autofix-workflow.test.js -t 'turns a budget breach into a growth-audit round' — passed
  • vitest scripts/tests/qwen-autofix-workflow.test.js -t 'runs cross-package autofix contracts' — passed
  • vitest scripts/tests/qwen-autofix-workflow.test.js (full file) — 229 passed (229)
  • mutation probes P1–P10 — every mutant turned its witness red, then restored to green
  • npm run test:scripts (full scripts suite) — three full runs failed 6 / 6 / 3 tests in shifting combinations; the only failure inside a file this PR touches was one 30s timeout of the unrelated stale-duplicate replay test, which passes in isolation (10.2s) and passed the next full run. Every other failing test lives in a file this PR does not touch (30s-timeout flat-config tests, one rendering-pixel assertion, one timing-sensitive upload assertion) and passes when re-run in isolation (5–14s vs the 30s ceiling). Both touched test files pass their complete runs (25/25 and 229/229) — load-induced flakiness on the shared pool, the exact class this PR clamps, not a regression from this diff.
中文说明

Autofix 评审第 3 轮 — PR #10671

共分诊 9 条行内反馈:本轮实现 4 条,重新验证 4 条(已由早前提交 8f96efe3c0 修复,变异探针重跑),拒绝 1 条并更正其记录。

本轮实现

[R2-2] 钉住 vitest-1.x 的 maxThreads 屏障,而不是依赖它。
scripts/tests/unit-vitest-configs.test.tsautofix gate load clamps)新增用例:从 lockfile 的嵌套副本推导 vitest-1.x 工作区(限定工作区目录;提升副本 >= 2 的前提本身也被钉住),并断言每个这样的工作区保持 threads 池和数值型 poolOptions.threads.maxThreads——tinypool 先读该值再读 ctx.config.maxWorkers,正是它让 --maxWorkers=25%(被 vitest 1.x 以 Number('25%') 强转为 NaN)不至于打崩该腿。vitest-1.x 工作区若不在配置登记表里,会以带指引的信息失败。已变异验证:从 packages/sdk-typescript/vitest.config.ts 移除 maxThreads: 4 会让该测试变红。

[R2-3] 钉住 export AUTOFIX_VITEST_FLAGS 这一行。 在现有赋值钉旁新增 toContain('export AUTOFIX_VITEST_FLAGS'),并加顺序断言:它必须位于 bash "${RUNNER_TEMP}/check-autofix-contracts.sh" 调用之前——run_check_no_ab 以子 bash 启动,只继承已导出的变量,export 缺失或晚于调用都会让 drift 腿静默退回 vitest 默认 5 秒。两个方向均已变异验证:删除 export 行、把 export 移到 contracts 调用之后,都会让结构钉变红。

[R2-4] 让假 npm 测试桩保留 argv 边界。 contracts 功能用例中的测试桩改为每个 argv 词一行带括号输出(printf '[%s]\n' "$@"),不再用 $* 拼接;同一 it 内四处期望同步改为逐词形式。已变异验证:"${vitest_flags[@]}""${vitest_flags[*]}" 变异体(参数以单个拼接词转发)会渲染为单个含空格的元素,使期望失败。

[R2-1] 把"issue-fix 腿不加降载"的相关注释更正为真实情况(该发现明确给出的第二个选项)。共四处:check-autofix-contracts.shrun-autofix-review-verification.sh(contracts 调用处注释)以及两条镜像测试注释。更正后写明:web-shell 配置不设任何超时、也没有 RUNNER_NAME 分支,因此 drift 测试在哪里都按 vitest 默认 5 秒运行;review gate 通过 AUTOFIX_VITEST_FLAGS 传入自身降载;issue-fix gate 与 repo-hygiene 的 docker 腿(此前未被提及的第三个调用方)不带该变量调用脚本,有意接受该默认值。另一个首选方案——在 issue-fix gate 步骤里也导出 AUTOFIX_VITEST_FLAGS——需要编辑本 PR 从未触碰的 .github/workflows/qwen-autofix.yml(还需在本 PR 足迹之外的区域为 review/issue-fix 两份副本补协调的结构钉),故选择注释更正来解决该发现的事实性问题,不做该扩张。

重新验证(由 8f96efe3c0 修复,本轮重跑探针)

[R1-3] 降载到达调用点的结构钉:从按包腿删除展开、从 bite 运行器删除展开、删除 AUTOFIX_VITEST_FLAGS 赋值、清空数组,均会让相应见证变红(探针 P5、P6、P7、P10)。

[R1-4] 降载/配置等价测试:把 shell 中的 --testTimeout 漂移为 61000 会使其变红(探针 P8)。

[R1-5] gate 脚本注释已是收窄后的声明("vitest 配置层"),并点名记录了残留群体;已确认在位。关闭残留本身(两个 env -i 白名单加 gate 哨兵 + 逐文件修改)仍是第 2 轮记录的另一切片。

[R1-6] 第三腿降载:删除 AUTOFIX_VITEST_FLAGS 赋值、或删除 check-autofix-contracts.sh 内的参数展开,都会让结构钉与功能用例变红(探针 P7、P9)。

拒绝

[R1-1] 确定性崩溃的指控在当前 head 上不成立——第 2 轮曾用完整降载数组在锁定的 vitest 1.6.1 sdk 套件上跑通(37 文件、1747 用例、exit 0),R2-2 自己的三臂见证也显示:有 sdk 的 maxThreads 屏障时该腿通过。但第 2 轮的理由是错的("两个主版本都接受的写法"——1.6.1 接受参数名,却把百分值强转成 NaN);线程上的回复已更正该记录,残余风险改由 R2-2 的屏障测试钉住,而不采用更大的按主版本派生 worker 参数方案。线程保持打开,以便维护者看到更正后的理由。

变异探针(每次探针后均已还原)

# 变异体 见证 结果
P1 从 sdk vitest 配置移除 maxThreads: 4 屏障测试 变红
P2 删除 export AUTOFIX_VITEST_FLAGS export 结构钉 变红
P3 把 export 移到 contracts 调用之后 顺序断言 变红
P4 ${vitest_flags[@]}${vitest_flags[*]}(拼接整体) contracts 用例 变红
P5 从按包腿删除降载展开 结构钉 1 变红
P6 从 bite 运行器删除降载展开 结构钉 2 变红
P7 删除 AUTOFIX_VITEST_FLAGS 赋值 结构钉 3 变红
P8 shell --testTimeout=6000061000 等价测试 变红
P9 删除 check-autofix-contracts.sh 内的参数展开 contracts 用例 变红
P10 清空 VITEST_LOAD_CLAMPS 数组 等价测试 变红

所有探针结束后 git status 仅显示预期的四个文件被修改;packages/sdk-typescript/vitest.config.ts 仅被 P1 瞬时触碰。

冲突说明

--conflict false;未执行合并。

验证

  • npm run build — 通过
  • npm run typecheck — 通过(屏障测试最终收窄后又跑了一遍)
  • npm run lint — 通过(同上又跑了一遍)
  • vitest scripts/tests/unit-vitest-configs.test.ts — 25 通过(25)
  • vitest scripts/tests/qwen-autofix-workflow.test.js -t 'turns a budget breach into a growth-audit round' — 通过
  • vitest scripts/tests/qwen-autofix-workflow.test.js -t 'runs cross-package autofix contracts' — 通过
  • vitest scripts/tests/qwen-autofix-workflow.test.js(整文件)— 229 通过(229)
  • 变异探针 P1–P10 — 每个变异体都让对应见证变红,还原后恢复全绿
  • npm run test:scripts(scripts 全套件)— 三轮完整运行分别失败 6 / 6 / 3 处,组合每轮不同;唯一落在本 PR 触碰文件内的失败是无关的 stale-duplicate 回放用例撞了 30 秒超时,单独重跑通过(10.2 秒),下一轮完整运行也通过。其余失败全部位于本 PR 未触碰的文件(30 秒超时的 flat-config 用例、1 个渲染像素断言、1 个计时敏感的上传断言),单独重跑全部通过(5–14 秒,上限 30 秒)。两个被修改的测试文件整文件运行均通过(25/25 与 229/229)——共享资源池负载导致的抖动,正是本 PR 要降载的那类问题,并非本 diff 引入的回归。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

🧵 Resolved all 8 selected review thread(s). · 已关闭全部选中的 8 条评审线程。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

Not reviewed: the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed.

Not reviewed: the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed.

Not explored to full depth (tool budget reached): "agent 6b": none — no check was cut short..

Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: scripts/tests/qwen-autofix-workflow.test.js (findings in round 2; 2 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

未审查:the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed。

未审查:the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed。

未探索到全部深度(达到工具调用预算):"agent 6b"none — no check was cut short.

收敛情况:第 3 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 4 条(其中 4 条首次提出)。发现反复回到同一批文件:scripts/tests/qwen-autofix-workflow.test.js(第 2 轮已出过发现,本轮又有 2 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff,除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上;并会一直如此,直到某一轮的标记重新带上锚点,或落地的嫁接能被运行该轮的评审使用。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.3)

expect(reviewVerificationRunner).toContain(
'AUTOFIX_VITEST_FLAGS="${VITEST_LOAD_CLAMPS[*]}"',
);
expect(reviewVerificationRunner).toContain('export AUTOFIX_VITEST_FLAGS');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R3-1: nothing pins the VITEST_LOAD_CLAMPS=(...) definition block above its consumers. The new pins are position-blind (the toContains here, and the parity regex in unit-vitest-configs.test.ts matches anywhere in the file), and only the export-vs-contracts-call ordering is checked — so a refactor that moves the array below its consumers leaves every pin green, while bash expands the then-unset array to zero words under the gate's set -eo pipefail without -u: AUTOFIX_VITEST_FLAGS becomes empty and the package and bite legs lose all four flags, silently reverting every gate leg to the incident conditions (15s timeouts, unbounded workers, coverage on) on the saturating shared host — the exact failure class this PR exists to prevent, returning with no red test in between.

Witness:

verifier mutation, scratch tree at 63fb0dcce:
INTACT:   Test Files 2 passed (2), Tests 254 passed (254)
MUTATED (array moved below all consumers):
          Test Files 2 passed (2), Tests 254 passed (254)   <- every pin stays green
bash probe: star-join of unset array -> [] (len=0), exit 0, no error
Suggested change
expect(reviewVerificationRunner).toContain('export AUTOFIX_VITEST_FLAGS');
expect(reviewVerificationRunner).toContain('export AUTOFIX_VITEST_FLAGS');
expect(
reviewVerificationRunner.indexOf('VITEST_LOAD_CLAMPS=('),
).toBeLessThan(
reviewVerificationRunner.indexOf(
'AUTOFIX_VITEST_FLAGS="${VITEST_LOAD_CLAMPS[*]}"',
),
);

The pin must be an explicit ordering check rather than reliance on any shell error: the gate script runs set -eo pipefail without -u (.github/scripts/run-autofix-review-verification.sh:2), so unset-array expansion is silent. If the fix is applied, moving the VITEST_LOAD_CLAMPS=(...) block below its consumers in the gate script must turn the new assertion red — please apply that mutation and confirm the test fails.

中文说明

R3-1:没有任何测试钉住 VITEST_LOAD_CLAMPS=(...) 定义块必须位于其消费者之前。新增的结构钉都是位置无关的(此处的 toContainunit-vitest-configs.test.ts 中的等价性正则在文件任意位置都能匹配),且只检查了 export 与 contracts 调用的顺序——因此把数组移到消费者下方的重构不会让任何结构钉变红,而 bash 在 gate 脚本 set -eo pipefail(无 -u)下会把未定义的数组静默展开为零个词:AUTOFIX_VITEST_FLAGS 变为空,按包测试腿与 bite 腿失去全部四个参数,每条 gate 测试腿静默退回事故状态(15 秒超时、worker 不限量、coverage 全开)——本 PR 要消除的那类故障在无一个测试变红的情况下回归。

(证据见英文区 Witness 代码块:完整树与"数组移到消费者下方"的变异体均 254/254 全绿;bash 探针确认未定义数组的 [*] 拼接为空且无报错。)

修复约束:结构钉必须是显式的顺序断言,不能依赖任何 shell 报错——gate 脚本以不带 -uset -eo pipefail 运行(.github/scripts/run-autofix-review-verification.sh:2),未定义数组的展开是静默的。若采纳修复:把 VITEST_LOAD_CLAMPS=(...) 块移到 gate 脚本中消费者下方时,新断言必须变红——请应用该变异并确认测试失败。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment on lines +8969 to +8971
expect(
reviewVerificationRunner.indexOf('export AUTOFIX_VITEST_FLAGS'),
).toBeLessThan(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R3-2: the pin suite pins the establish side of the AUTOFIX_VITEST_FLAGS transport (assignment, export, and this export-before-call ordering) but never the remove side — unset AUTOFIX_VITEST_FLAGS is pinned nowhere in scripts/tests/ (grep returns zero matches). The drift leg receives the clamps only through the environment at child-spawn time (run_check_no_abstrip_runner_channels preserves the export → check-autofix-contracts.sh:25 reads it), so moving the unset above the contracts call leaves the child inheriting no variable: read -r -a yields an empty array, the guarded expansion yields nothing, and the web-shell drift test runs at vitest's 5s default on the saturating host — with every pin here still green. The behavioral fixture test does not catch this either: it injects AUTOFIX_VITEST_FLAGS directly into the child env, bypassing the export/unset lifecycle. This is the fourth silent shape of the hazard class the comment above this block names.

Witness:

probe, scratch tree at 63fb0dcce (mutant: unset moved above the contracts call):
shipped pin suite:            Tests 1 passed   <- regression survives
same mutant, pin below added: AssertionError: expected 34333 to be less than 34241
intact tree with the pin:     green

Add the symmetric ordering pin after this block:

expect(
  reviewVerificationRunner.indexOf(
    'bash "${RUNNER_TEMP}/check-autofix-contracts.sh"',
  ),
).toBeLessThan(
  reviewVerificationRunner.indexOf('unset AUTOFIX_VITEST_FLAGS'),
);

The unset must stay after run_check_no_ab 'cross-package contract verification failed': check-autofix-contracts.sh:25 reads the variable from its inherited environment at spawn time, reached through strip_runner_channels' env -u … (.github/scripts/run-autofix-review-verification.sh:503). If the fix is applied, moving unset AUTOFIX_VITEST_FLAGS above the contracts call — or deleting it outright — must turn the new pin red — please apply that mutation and confirm the test fails.

中文说明

R3-2:结构钉套件钉住了 AUTOFIX_VITEST_FLAGS 传递链的建立侧(赋值、export、以及这里的 export 先于调用),却从未钉住移除侧——scripts/tests/ 中没有任何测试钉住 unset AUTOFIX_VITEST_FLAGS(grep 零匹配)。drift 测试腿只在子进程产生时通过环境变量获得降载参数(run_check_no_abstrip_runner_channels 保留已导出变量 → check-autofix-contracts.sh:25 读取),因此把 unset 移到 contracts 调用上方会让子进程继承不到该变量:read -r -a 得到空数组,带守卫的展开什么也不传,web-shell 的 drift 测试在饱和主机上退回 vitest 默认 5 秒——而这里的所有结构钉依旧全绿。行为夹具测试同样抓不到:它把 AUTOFIX_VITEST_FLAGS 直接注入子进程环境,绕过了 export/unset 生命周期。这是上方注释所列隐患类别的第四种静默形态。

(证据见英文区 Witness 代码块:变异体(把 unset 移到 contracts 调用上方)下现有钉套件 Tests 1 passed——回归存活;加上下方建议的结构钉后变红;完整树加新钉为绿。)

修复约束:unset 必须留在 run_check_no_ab 'cross-package contract verification failed' 之后:check-autofix-contracts.sh:25 在子进程产生时从继承的环境读取该变量,经由 strip_runner_channelsenv -u …(.github/scripts/run-autofix-review-verification.sh:503)。若采纳修复:把 unset AUTOFIX_VITEST_FLAGS 移到 contracts 调用上方(或直接删除)必须让新结构钉变红——请应用该变异并确认测试失败。

— qwen3.8-max via Qwen Code /review (v0.22.3)

…sport

Addresses review round 3 on QwenLM#10671; both pins mutation-verified.

R3-1 Nothing pinned the VITEST_LOAD_CLAMPS definition above its
consumers: the existing pins are position-blind (toContain here, the
parity regex in unit-vitest-configs.test.ts matches anywhere), so
moving the array below its consumers left every pin green while bash
expanded the then-unset array to zero words under the gate's
`set -eo pipefail` without `-u` — AUTOFIX_VITEST_FLAGS goes empty and
the package and bite legs lose all four clamps, silently reverting to
the incident conditions. Added an explicit ordering pin against the
star-join, the first consumer in script order, which pins the
definition above every consumer. Outright deletion was already caught
by the parity test's existence assertion; the move was the only
surviving hole. Move mutant verified red (61668 < 34052 fails).

R3-2 The remove side was pinned nowhere: moving `unset
AUTOFIX_VITEST_FLAGS` above the contracts call (or deleting it)
strips the export the drift leg inherits at child-spawn time, leaving
the web-shell drift test at vitest's 5s default with every
establish-side pin green. Added the symmetric ordering pin, contracts
call before unset. Move and delete mutants both verified red
(34333 < 34241 and 34306 < -1 fail).
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下:

Feedback addressed

R3-1 (rc:3904831960) — nothing pins the VITEST_LOAD_CLAMPS=(...) definition block above its consumers — RESOLVED

The claim was reproduced on the round's head before any change: moving the array definition below all consumers in the gate script left the full pin suite green (2 files, 254/254 tests), and a bash probe confirmed the gate's set -eo pipefail without -u expands the then-unset array to zero words — AUTOFIX_VITEST_FLAGS goes empty and the package and bite legs lose all four clamps, silently reverting to the incident conditions. Fixed with an explicit ordering pin: indexOf('VITEST_LOAD_CLAMPS=(') must precede indexOf('AUTOFIX_VITEST_FLAGS="${VITEST_LOAD_CLAMPS[*]}"') — the star-join is the first consumer in script order, so the definition is pinned above every consumer. Outright deletion of the block was already caught by the parity test's existence assertion in unit-vitest-configs.test.ts; the move was the only surviving hole. Witness: the move mutant turns the new pin red (AssertionError: expected 61668 to be less than 34052); restored tree green.

R3-2 (rc:3904831981) — the remove side of the AUTOFIX_VITEST_FLAGS transport is pinned nowhere — RESOLVED

The claim was reproduced: moving unset AUTOFIX_VITEST_FLAGS above the contracts call left the suite green (254/254), so the drift-leg child spawns without the variable and the web-shell drift test reverts to vitest's 5s default. Added the symmetric ordering pin: the contracts call must precede unset AUTOFIX_VITEST_FLAGS. Witnesses: the move mutant fails red (AssertionError: expected 34333 to be less than 34241); the delete-outright mutant fails red (AssertionError: expected 34306 to be less than -1); restored tree green in both cases.

Review body rv:5078930354 — noted, no action

Informational disclosures only: shellcheck not installed for the two gate scripts on the review host, a tool-budget note, and a convergence observation explicitly marked "Observation only". The two actionable items it carried are the inline findings above, both resolved.

Conflict notes

None — --conflict false; no merge performed.

Verification

  • Baseline focused Vitest (pre-change): npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js scripts/tests/unit-vitest-configs.test.ts — 2 files, 254 tests passed
  • Reproduction probes (pre-fix mutants): array moved below all consumers → 254/254 green (gap confirmed); unset moved above the contracts call → 254/254 green (gap confirmed); bash probe confirmed unset-array expansion is silent under set -eo pipefail without -u
  • Mutation witnesses (post-fix): definition moved below consumers → new pin red (expected 61668 to be less than 34052); unset moved above the contracts call → new pin red (expected 34333 to be less than 34241); unset deleted outright → new pin red (expected 34306 to be less than -1); restored tree green after each mutant
  • Focused Vitest (post-change): npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js scripts/tests/unit-vitest-configs.test.ts — 2 files, 254 tests passed
  • npm run test:scripts (full scripts suite) — 2017 passed, 16 skipped; 5 failures in files this PR never touches, reproduced identically with this round's change reverted (tree byte-identical to pre-round HEAD): 4 proxy/tmux tests in qwen-triage-workflow.test.js (localhost socket binds fail in this sandbox) and 1 terminal-color test in verify-capture.test.js; both files are byte-identical to origin/main — pre-existing environment-specific failures, not introduced by this round
  • npm run build — passed (exit 0)
  • npm run typecheck — passed (exit 0)
  • npm run lint — passed (exit 0)
  • npx prettier --check scripts/tests/qwen-autofix-workflow.test.js — passed
  • No integration tests run: the change adds structural pins over already-committed gate-script text; nothing in the change is exercised only through the bundled CLI or the integration harness
  • No settings source changed, so npm run generate:settings-schema was not needed
中文说明

已处理的反馈

R3-1 (rc:3904831960) — 没有任何测试钉住 VITEST_LOAD_CLAMPS=(...) 定义块必须位于其消费者之前 — 已解决

该主张在做任何改动之前已在本轮头提交上复现:把数组定义移到 gate 脚本中所有消费者下方后,整个结构钉套件依旧全绿(2 个文件,254/254 个测试通过);bash 探针确认 gate 的 set -eo pipefail(无 -u)会把未定义数组静默展开为零个词——AUTOFIX_VITEST_FLAGS 变为空,按包测试腿与 bite 腿失去全部四个降载参数,静默退回事故状态。修复:新增显式顺序结构钉,要求 indexOf('VITEST_LOAD_CLAMPS=(') 必须位于 indexOf('AUTOFIX_VITEST_FLAGS="${VITEST_LOAD_CLAMPS[*]}"') 之前——星号拼接是脚本顺序中的第一个消费者,因此定义被钉在所有消费者之上。整块删除此前已由 unit-vitest-configs.test.ts 中平价测试的存在性断言捕获;移动是唯一存活的漏洞。证据:移动变异体使新钉变红(AssertionError: expected 61668 to be less than 34052);还原后为绿。

R3-2 (rc:3904831981) — AUTOFIX_VITEST_FLAGS 传递链的移除侧没有任何结构钉 — 已解决

该主张已复现:把 unset AUTOFIX_VITEST_FLAGS 移到 contracts 调用上方后,套件依旧全绿(254/254),漂移腿子进程在产生时继承不到该变量,web-shell 漂移测试退回 vitest 默认 5 秒。修复:新增对称的顺序结构钉,要求 contracts 调用必须位于 unset AUTOFIX_VITEST_FLAGS 之前。证据:上移变异体变红(AssertionError: expected 34333 to be less than 34241);直接删除变异体变红(AssertionError: expected 34306 to be less than -1);两种情况还原后均为绿。

评审正文 rv:5078930354 — 已知悉,无需处理

仅信息性披露:评审主机未安装 shellcheck、两个 gate 脚本未做可执行脚本 lint;工具预算说明;以及明确标注"仅为观察"的收敛观察。其中可执行的两项即上面的行内发现,均已解决。

冲突说明

无——--conflict false,未执行合并。

验证

  • 基线聚焦 Vitest(改动前):npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js scripts/tests/unit-vitest-configs.test.ts — 2 个文件,254 个测试通过
  • 复现探针(修复前变异体):数组移到所有消费者下方 → 254/254 全绿(缺口确认);unset 移到 contracts 调用上方 → 254/254 全绿(缺口确认);bash 探针确认 set -eo pipefail(无 -u)下未定义数组的展开是静默的
  • 变异证据(修复后):定义移到消费者下方 → 新钉变红(expected 61668 to be less than 34052);unset 移到 contracts 调用上方 → 新钉变红(expected 34333 to be less than 34241);直接删除 unset → 新钉变红(expected 34306 to be less than -1);每个变异体还原后均为绿
  • 聚焦 Vitest(改动后):同上命令 — 2 个文件,254 个测试通过
  • npm run test:scripts(scripts 全套)— 2017 通过,16 跳过;5 个失败位于本 PR 从未触碰的文件,且在本轮改动被还原(树与轮前 HEAD 逐字节一致)后同样复现:qwen-triage-workflow.test.js 中 4 个 proxy/tmux 测试(本沙箱中本地端口绑定失败)与 verify-capture.test.js 中 1 个终端颜色测试;两个文件与 origin/main 逐字节一致——属既有环境性失败,非本轮引入
  • npm run build — 通过(exit 0)
  • npm run typecheck — 通过(exit 0)
  • npm run lint — 通过(exit 0)
  • npx prettier --check scripts/tests/qwen-autofix-workflow.test.js — 通过
  • 未运行集成测试:本次改动只是对已提交的 gate 脚本文本新增结构钉,没有任何内容只能通过打包 CLI 或集成测试框架执行
  • 未触碰任何 settings 源,因此无需 npm run generate:settings-schema

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

⚠️ Review-thread resolution skipped — guard: live-head drift; resolved 0 of 2 selected thread(s), 2 left for a later round. · 评审线程关闭被跳过——守卫:live-head drift;选中 2 条,本轮关闭 0 条,其余 2 条留待后续轮次。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI still running. Partially reviewed — gaps disclosed.

Not reviewed: the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed.

Not reviewed: the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed.

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

⚠️ 已从批准降级为评论:CI still running。 仅完成部分审查,审查缺口已披露。

未审查:the executable-script lint — .github/scripts/check-autofix-contracts.sh: shellcheck is not installed。

未审查:the executable-script lint — .github/scripts/run-autofix-review-verification.sh: shellcheck is not installed。

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff,除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上;并会一直如此,直到某一轮的标记重新带上锚点,或落地的嫁接能被运行该轮的评审使用。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.3)

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking findings.
Approval blockers: none.

Checked:

  • Array expansion correctness${VITEST_LOAD_CLAMPS[@]} used directly in two gate legs; round-trip via ${VITEST_LOAD_CLAMPS[*]}read -r -a for the subprocess leg (check-autofix-contracts.sh). All four flag values (--maxWorkers=25%, --testTimeout=60000, --hookTimeout=60000, --coverage.enabled=false) contain no IFS characters, so the serialization is lossless.
  • Empty-array guard${vitest_flags[@]+"${vitest_flags[@]}"} in check-autofix-contracts.sh is the correct bash idiom; no set -u hazard.
  • Ordering constraints — the four indexOf position tests in qwen-autofix-workflow.test.js correctly pin: CLAMPS definition before AUTOFIX_VITEST_FLAGS assignment, export before the contracts bash call, unset after it. Silent empty-array expansion (set -eo pipefail without -u) is the exact failure mode these tests guard against, and it is called out in the comment.
  • npm stub $*$@ — bracket-per-arg logging makes word-splitting bugs observable; test expectations updated correctly throughout.
  • ECS parity testunit-vitest-configs.test.ts re-imports configs under a stubbed RUNNER_NAME env var and regex-parses the shell script to cross-verify values. The regex /^VITEST_LOAD_CLAMPS=\(\n([\s\S]*?)\n\)$/m and the subsequent split/strip parse are correct for the current indented format.
  • vitest 1.x shield — the test that pins poolOptions.threads.maxThreads for any legacy vitest 1.x workspace is present; the NaN-coercion failure mode (Number('25%') → NaN → new Array(NaN): RangeError) is correctly diagnosed.

Scope / not reviewed:

  • shellcheck static lint (not installed in this environment) — bots flagged this gap too; no evidence of a regression from the diff alone.

Acknowledged residual (not a blocker, explicitly deferred by author): 5 test files that set their own ceiling via vi.setConfig keyed on RUNNER_NAME remain unclamped inside the gate. The PR description names them and labels this a separate follow-up slice.

Reviewed with AI assistance.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head a720f125.

  • The fix addresses a real hole: the autofix gate runs vitest through an env -i allowlist that drops RUNNER_NAME, silently deactivating every config's ECS clamp branch — passing the clamps explicitly (VITEST_LOAD_CLAMPS → the changed-package loop and, via AUTOFIX_VITEST_FLAGS, the contracts drift leg, exported and unset around the call) closes it deterministically, and the parity test pins the flags against the configs' own ECS branches.
  • Re-checked the one Critical from review: --maxWorkers=25% reaching vitest 1.x does not crash — reproduced against the nested vitest 1.6.1 locally, which parses the unknown options and exits 0 — and the residual (the clamp being inert there) is pinned by the dedicated vitest-1.x thread-cap ratchet in unit-vitest-configs.test.ts. The two later Suggestions (position-blind pins, unpinned unset transport side) were addressed by the last two commits; the three open threads are bookkeeping, their substance is resolved at this head.
  • CI on this head has no failures (one web-shell job still running); yiliang114's earlier Approve covered the body of this change. No new Critical issues found; per the channel convention the call is on the review itself.

@yiliang114
yiliang114 added this pull request to the merge queue Sep 1, 2026
Merged via the queue into QwenLM:main with commit 09958e0 Sep 1, 2026
78 of 82 checks passed

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review passed.

  • Root cause is real and the fix removes it from env plumbing: the gate runs through the env -i allowlist that drops RUNNER_NAME, so the vitest configs' ECS clamps silently deactivate in there. Passing --maxWorkers=25% --testTimeout=60000 --hookTimeout=60000 --coverage.enabled=false explicitly on all three gate-launched vitest legs (per-package --changed run, bite runner, and the web-shell drift test via AUTOFIX_VITEST_FLAGS) makes the verdict independent of the env layer; coverage-off is justified since nothing in the gate/report path consumes it.
  • Transport is safe in both caller modes: verified the read -r -a ... <<< ${AUTOFIX_VITEST_FLAGS:-} + ${arr[@]+"${arr[@]}"} idiom locally — unset expands to zero words (issue-fix gate / repo-hygiene keep their defaults), set splits correctly.
  • Residuals are honestly documented: the vi.setConfig files that outrank CLI flags are named and deferred as a separate slice; the vitest-1.x --maxWorkers NaN hazard is shielded by the pinned numeric threads cap.
  • Regression pins are unusually thorough: clamp values must track the configs' ECS branch, array definition/export/unset ordering is positionally pinned (definition above consumers, export above the child spawn, unset below it), and the npm mock now logs one bracketed line per argv word so a [*]-for-[@] regression cannot hide.

Verified locally on a720f12: unit-vitest-configs 25/25, contracts transport test 1/1 (both flag modes), growth-audit runner pins 3/3.

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

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants