Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yiliang114 merged 6 commits into from
Sep 1, 2026
Merged

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

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
041d8c4
fix(autofix): clamp gate test load explicitly instead of via RUNNER_NAME
qqqys Sep 1, 2026
d8c42f0
Merge branch 'main' into fix/autofix-gate-load-clamps
yiliang114 Sep 1, 2026
8f96efe
fix(autofix): clamp the gate's third vitest leg and pin the clamps
qqqys Sep 1, 2026
325bdf4
Merge remote-tracking branch 'origin/fix/autofix-gate-load-clamps' in…
qqqys Sep 1, 2026
63fb0dc
fix(autofix): pin the clamp witnesses and correct the unclamped-leg r…
qqqys Sep 1, 2026
a720f12
test(autofix): pin both sides of the gate's AUTOFIX_VITEST_FLAGS tran…
qqqys Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions .github/scripts/run-autofix-review-verification.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,25 @@ run_check_no_ab 'lint failed on the agent-committed fix' npm run lint
# Test changed/related files for the packages this PR touches.
# --changed follows the import graph so transitive breakage is caught.
# Full regression is covered by regular CI on the PR after the push.
#
# The gate launches through an env -i allowlist that (deliberately) drops
# RUNNER_NAME, so the vitest configs' ECS load clamps (60s test/hook
# timeouts, maxWorkers 25%) silently deactivate in here: tests run with
# 15s timeouts and unbounded workers on a host shared with other autofix
# jobs. Under pool saturation that produced both false rejections (73
# load-induced 15s timeouts charged to a round on #10171) and gate deaths
# past the step's 60-minute cap that discarded verified fixes (#10171
# rounds 1/2/5-7, #10543 x5). Pass the same clamp values explicitly —
# the verdict must not depend on env plumbing or runner naming — and turn
# coverage off: nothing in the gate consumes it, and its collection was
# the bulk of the overrun. Applies to every vitest invocation below (the
# per-package --changed run and the bite check).
VITEST_LOAD_CLAMPS=(
--maxWorkers=25%
--testTimeout=60000

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

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 结构钉不受影响。线程保持打开，以便维护者看到更正后的理由。

--hookTimeout=60000
--coverage.enabled=false
)
# Map each changed file to its OWNING npm workspace via the trusted
# staged resolver, shared with the other verify gate so both resolve
# packages identically. It expands the on-disk root package.json
Expand Down Expand Up @@ -1038,7 +1057,7 @@ else
# npm exits 1 there with "No workspaces found".) Their rejections stay
# charged to the round, where the repair agent can act.
run_check_no_ab "tests failed in ${p}" \
npm run test --workspace "${p}" --if-present -- --changed origin/main --passWithNoTests
npm run test --workspace "${p}" --if-present -- --changed origin/main --passWithNoTests "${VITEST_LOAD_CLAMPS[@]}"
done
fi

Expand Down Expand Up @@ -1086,7 +1105,7 @@ bite_runner_default() {
# $1 = workspace dir, rest = test paths relative to the workspace.
local ws="${1}"
shift
strip_runner_channels npm run test --workspace "${ws}" --if-present -- "$@"
strip_runner_channels npm run test --workspace "${ws}" --if-present -- "${VITEST_LOAD_CLAMPS[@]}" "$@"
}
mapfile -d '' -t BITE_FILES < <(git diff --name-only -z --no-renames --diff-filter=AM "${ROUND_RANGE}" \
-- ':(glob)**/*.test.*' ':(glob)**/*.spec.*' ':(exclude,glob)**/__snapshots__/**' \
Expand Down
Loading