ci: give seconds-long jobs their own ECS lane - #10575
Conversation
|
Re-running the gate on Template: complete ✓ — What / Why / Reviewer Test Plan / Risk & Scope plus the Chinese translation; "Verification notes" stands in for Evidence, reasonable for a CI-only change, and no linked issue is needed here. Problem: observed and measured, not theoretical — 121 jobs queued on Direction: aligned — the repo's own CI plumbing, and the unblocked jobs include gates ( Size: not applicable — no core package paths; everything is under Approach: right scope. The exclusions ( Risk: no elevated risk signals — none of the revert-correlated paths are touched. Moving on to code review. 🔍 中文说明在 模板:完整 ✓ —— What / Why / Reviewer Test Plan / Risk & Scope 及中文翻译;"Verification notes" 替代 Evidence 小节,对纯 CI 改动合理,也无需关联 issue。 问题:已观测、有测量,不是理论问题——2026-08-30 饱和期间 方向:对齐——是仓库自身的 CI 基建,被解阻塞的还包括 规模:不适用——未触及任何核心包路径,全部在 方案:范围合理。刻意排除项( 风险:无升级风险信号——未命中任何与 revert 相关的路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Independent baseline first: from the title and motivation alone, my proposal is the same shape as before — carve a dedicated self-hosted lane for the seconds-long jobs, keep every routing guard and the kill-switch intact, and register the lanes so a workflow cannot ask for a label the fleet doesn't carry. The PR matches that baseline; the final commit addresses the two Criticals the last review round left standing. Findings:
Testing evidence — this PR's own CI, read via the API (unattended run; no PR code executed): on Not verified, and honest about it: neither sandboxed lane can observe runner scheduling — CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 中文说明先看独立方案:仅凭标题和动机,我的方案与此前一致——给秒级 job 单独分一条自建池 lane,保留全部路由守卫与 kill-switch,并注册 lane 清单使 workflow 无法请求机群未携带的标签。PR 与该基线一致;头提交解决了上一轮评审遗留的两个 Critical。 要点:
测试证据:无人值守运行,通过 API 读取本 PR 自身的 CI(未执行任何 PR 代码)。 未验证、如实说明:两个沙箱通道都观测不到 runner 调度—— — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — every blocking item from six review rounds and one human review is structurally closed and pinned at this head; what remains is the rounds' deferred non-blocking ledger and a fleet precondition a human already verified against the runners API. Stepping back: the lane swap itself stayed minimal through all of it — eight label changes that preserve every guard condition, one Reservations, named for the record: the guard's deferred ledger (empty On the standing review state: my approval here supersedes the bot's earlier request-changes reviews, which were tied to the commits they were written against. The human request-changes review from the CI on this commit is complete, so approval is given now, pinned to the reviewed commit. ✅ 中文说明置信度:4/5 —— 六轮评审与一次人类评审提出的所有阻塞项在该头提交上均已结构性闭合并钉住;剩下的只有各轮记录在案的非阻塞账目,以及一个已由人类评审者通过 runners API 核验过的机群前置条件。 整体来看:lane 迁移本身始终保持最小——八处保留全部守卫条件的标签替换、一条 保留意见,记录在案:守卫的延后账目(空 关于当前评审状态:我此处的批准取代机器人早前的请求修改评审——那些评审绑定在它们写下的提交上。 该提交上的 CI 已完成,因此现在给出批准,并钉在所审提交上。✅ — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1c": could not execute the vitest scripts/tests suite (no node_modules in this worktree); instead statically enumerated every runs-on /lane assertion in it and ….
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 1c":could not execute the vitest scripts/tests suite (no node_modules in this worktree); instead statically enumerated every runs-on /lane assertion in it and …。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2168 passed · 0 failed · 2168 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:2168 通过 · 0 失败 · 2168 总计 抖动门:✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 10575 deep verification —
|
| cell class | n | base arm (measured) | head arm (measured) |
|---|---|---|---|
| Trusted contexts, kill-switch off → pool lane | 6 unguarded × 2 + 2 guarded × 1 = 14 | ecs-qwen 14/14 |
ecs-light 14/14 |
Kill-switch MAINTAINER_ECS_RUNNER_DISABLED=true → hosted |
8 | ubuntu-latest 8/8 |
ubuntu-latest 8/8 |
Fork-PR and no-PR contexts on the 2 fork-guarded authorize jobs → hosted |
2 × 2 | ubuntu-latest 4/4 |
ubuntu-latest 4/4 |
| Head runs-on == base with exactly the label swapped (T2 + T2b) | 8 + 8 | — | 16/16 |
T1: every base job exists at head, and non-moved jobs' runs-on is byte-identical (decoys) / T1c: no head job absent from base |
30 + 19 | — | 49/49 |
| Whole document deep-equal after normalizing the 8 runs-on back (T3) | 6 | — | 6/6 |
HELPER_TESTS wiring in ci.yml (T6): entry listed, equals base + entry, file exists, both consumers serial |
4 | — | 4/4 |
| Timeout census pinned (T7) | 8 | — | 8/8 (values in F1) |
T4/T5 = 26 cells per arm (6 unguarded jobs × 3 contexts + 2 fork-guarded jobs × 4 contexts); full log ab-routing.log, 135 pass / 0 fail.
The fork-guard subtlety is preserved: qwen-code-pr-review.yml/authorize and qwen-triage.yml/authorize keep their github.event.pull_request && head.repo.full_name == github.repository clause; ack-review-request and review-config never had one — identical on both arms (see F3).
Guard load-bearing proof (guard-controls.mjs, witness 02-guard-controls-negative-matrix.png, 14/14):
| cell | result |
|---|---|
| G1 guard on head workflows | green, 38 tests |
G2 one job → unregistered ecs-lightning |
red: "ecs-lightning" is not a registered lane |
G3 every ecs-light reference reverted (PR's negative control 2) |
red: "ecs-light" is registered here but no workflow asks for it |
| G4 real base tree workflows (A/B control) | red, same orphaned-lane message |
| G8 one runs-on naming two lanes | red: expected one lane label beside the platform labels |
| G7 control (quoted bogus lane caught) | red — the harness can fail |
Repo's own tests, both arms (witnesses 03-testplan-step1-61-of-61.png, 04-helper-tests-head-vs-base.png, 05-vitest-mirrors-head.png):
| suite | head | base | delta |
|---|---|---|---|
Test-plan step 1 (ci-runner-routing + runner-lane-labels) |
61/61 | n/a (guard is new) | matches the body's claim exactly |
| HELPER_TESTS (full env list from each arm's ci.yml) | 533/533 | 495/495 | +38 pass, +0 fail — the +38 is exactly the new guard file's test count |
| vitest mirrors covering the changed surface (11 files: triage, pr-review, force-push, self-report-label, triage-finalize, ci-platform-lanes, workflow-size, capture-tmux-ci, no-ak, worktree-cleanup, serve-ab) | 687/687 | — | |
qwen-autofix-workflow.test.js (pins HELPER_TESTS membership) |
229/229 | — |
The author's verification note (7 HELPER_TESTS files failing on a bare worktree, 36 failures in qwen-triage-workflow.test.mjs "on both sides") does not reproduce in this container, which has node_modules: both arms are fully green. The "no new failures" claim holds here in its strongest form (0 = 0).
Findings
F1 — Suggestion: the Risk section's timeout bound contradicts the code; review-config has no timeout at all
The body's Risk & Scope says: "Every moved job carries timeout-minutes: 5 or less, which bounds that." Measured from the head YAML (T7 census, witness 06-size-gate-and-timeout-census.png):
| job | timeout-minutes |
|---|---|
| remind-on-force-push, label, ack-review-request, authorize (both) | 5 |
| finalize (qwen-triage-finalize.yml) | 10 |
| publish (serve-ab-publish.yml) | 10 |
| review-config (qwen-code-pr-review.yml) | none — GitHub's 360-minute default |
Reproduce: node -e "const {parse}=require('yaml');const fs=require('fs');const d=parse(fs.readFileSync('.github/workflows/qwen-code-pr-review.yml','utf8'));console.log(d.jobs['review-config']['timeout-minutes'])" → undefined.
Impact: the worst-case occupancy of one of the three light slots by a wedged job is bounded at 360 minutes (review-config) and 10 minutes (finalize/publish), not 5. The lane degrades (2 of 3 slots remain) rather than deadlocks, and the missing timeout is pre-existing at base — but this PR's risk argument explicitly rests on the ≤5-minute bound, so a reviewer approving on that basis approves a number 72× too small for one job.
Suggested fix (measured, not eyeballed — fixcheck.mjs, 8/8): add timeout-minutes: 5 to review-config and correct the body sentence. Applied in a scratch copy and driven through the same harnesses: (a) the parsed workflow differs from head only by review-config.timeout-minutes = 5; (b) the lane guard stays green on the patched tree; (c) all routing cells are byte-identical. Note no suite pins this axis today — the mirror suites are green with and without the fix — so if it lands, pin it, e.g. in scripts/tests/qwen-pr-review-workflow.test.js: expect(doc.jobs['review-config']['timeout-minutes']).toBeLessThanOrEqual(5).
F2 — Suggestion: the guard's matcher silently skips unquoted and block-sequence runs-on spellings
The guard extracts labels only from quoted bracket sets on single-line runs-on: values. Measured with probe workflows (guard-controls.mjs G5/G6, witness 02-guard-controls-negative-matrix.png):
runs-on: [self-hosted, linux, x64, ecs-bogus-unquoted](unquoted flow sequence) → guard green, invented lane passes;- block sequence (
runs-on:+- self-hosted…) with an invented lane → guard green; - G7 control (the quoted
fromJSONspelling with a bogus lane) → guard red, so the green results above are coverage gaps, not a dead harness.
Bounding: a full census of all 114 runs-on: values in .github/workflows/ shows every current value uses a covered spelling (single-line quoted expression or quoted flow sequence), so nothing slips through today — this is a latent gap, severity Suggestion. But the guard's own comment claims it "matches both spellings a runs-on can take", and GitHub accepts more than two. Minimal direction: add a census pin inside the guard that fails when any runs-on: value is empty after the key (block sequence) or contains a bracketed self-hosted set with zero extractable quoted labels — that converts the silent skip into a loud review prompt without widening the regex.
F3 — Informational (no action for this PR): ack-review-request and review-config admit fork-triggered events onto the pool
Neither carries the fork clause in runs-on, so fork-PR events route them to the self-hosted lane. This is pre-existing: the base arm resolves identically (T5 cells), and the PR swaps only the label. Noted because a reader of the diff may otherwise attribute it to this PR.
Not covered
- Fleet-side facts — that
ecs-lightis actually registered on the three runners (64c / sg / hk-j6cdq), the 5-host / 125-registration layout, and the queue metrics in the body (121 queued, 27-min median, 71%/13% split, the 3085 sauthorizesample): all live outside this container (labels live in runner configuration; there is no token here). The PR states the label was registered before this change; the first post-merge runs are the proof (test-plan step 4). - Test-plan step 4 (post-merge observation of
authorize/labelstarting within seconds on the light runners) — post-merge by definition. Steps 1–3 were all executed (tables above; step 2 as the G2/G3 cells, step 3 as the size gate). - Per-commit attribution — the metadata snapshot lists 2 commits (
fac151773d,7040dbbaaa); the depth-2 merge-ref checkout makes only the aggregateHEAD^1..HEADdiff reachable, so commit 2's comment-wording change is verified only as part of the aggregate. - yamllint — cannot be installed in this container (
pip3absent/permission-denied; environmental, not PR-caused). actionlint 1.7.12 (pinned byscripts/lint.js) ran clean over all workflows and was proven live (a planted broken expression fails it), covering workflow YAML/expression validity. - Repo-wide unit/integration suites and packages workspaces — the diff touches only
.github/workflows/*and onenode:testhelper; no production package code changed, so targeted gates were run instead. - The other short-ish jobs that stayed on
ecs-qwen(e.g.stale.yml,comment-attachment-guard.yml,repo-hygiene.yml) — a scope choice the body states; not re-litigated here.
Methodology
Environment: this CI verify container (Debian 12 bookworm, node v22.23.2 — the lane's own runtime; credential-free, npm ci + npm run build already done at HEAD; runner identity env vars are not exposed inside the container, so the host could not be named). Base side: git worktree add tmp/base-tree HEAD^1, reusing the root node_modules — a clean control because the diff touches no package.json/package-lock.json; the one shared dependency (yaml) resolves from the base tree to /__w/qwen-code/qwen-code/node_modules/yaml (realpath verified, a real directory, not a workspace symlink). Harnesses (ab-routing.mjs, guard-controls.mjs, fixcheck.mjs, this directory) drive the real workflow files and the real guard test via node --test in scratch copies; expression evaluation mirrors the repo's own evalRunsOn with an added github.event.pull_request truthiness substitution and a fail-loud residue check. Gates: check-workflow-size.sh with WORKFLOW_SIZE_BASE_SHA=HEAD^1 (per-file growth: +47 B ci.yml, +369 B per single-hunk file, +1107 B pr-review, all within the 4096 allowance; the larger baseline deltas on ci.yml/qwen-triage.yml are pre-existing drift), actionlint with a planted-violation liveness proof, prettier --check and eslint --max-warnings 0 on the changed files (eslint liveness-proven with a planted unused variable). Raw per-cell logs: ab-routing.log, guard-controls.log, testplan-step1.log, helpers-head.log, helpers-base.log, vitest-head.log, vitest-autofix-head.log, size-gate.log. The base worktree was removed after the cells were captured; git status is clean.
Flakiness gate log
rounds=5 files=1 skipped=0
file .github/scripts/ci/runner-lane-labels.test.mjs: (cd .) node --test ./.github/scripts/ci/runner-lane-labels.test.mjs
per-file results (P=pass F=fail I=infra-exit, one letter per run):
.github/scripts/ci/runner-lane-labels.test.mjs: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/ci/runner-lane-labels.test.mjs: P (exit 0)
round 2 · .github/scripts/ci/runner-lane-labels.test.mjs: P (exit 0)
round 3 · .github/scripts/ci/runner-lane-labels.test.mjs: P (exit 0)
round 4 · .github/scripts/ci/runner-lane-labels.test.mjs: P (exit 0)
round 5 · .github/scripts/ci/runner-lane-labels.test.mjs: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Review follow-ups on #10575: - The guard silently skipped any runs-on spelling its parser could not read — a block-sequence runs-on captures an empty value, an unquoted flow sequence yields no quoted tokens — so either spelling could ship an unregistered lane past a green suite, the exact incident class the guard exists to prevent. selfHostedLabelSets now returns those values as unreadable and the suite fails on each, naming the file and the spelling; a synthetic-text describe pins the contract for both spellings plus bare `runs-on: self-hosted` and block scalars. Verified end to end: a planted probe workflow with both invented-lane spellings turns the corpus scan red (2 failures), and removing the fail-closed block lets the same probe pass green again. - The eight workflow copies of the lane rationale still said "runner 1 on each ECS host", which the per-host split superseded — and eight copies of fleet topology is how they went stale in the first place. Each is now a one-line pointer to the registry header in runner-lane-labels.test.mjs, the single home of the convention. - review-config had no timeout-minutes; on a three-slot lane an uncapped job is a third of the lane if it wedges. Capped at 5 like its siblings.
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.3)
yiliang114
left a comment
There was a problem hiding this comment.
Independent pass at 85d33d7. The lane move itself is verified clean: all eight jobs swap only the lane label inside the existing conditional expression — repo guard, MAINTAINER_ECS_RUNNER_DISABLED kill-switch, fork same-repo guards, and the ubuntu-latest fallback are all preserved. I checked the fleet against the live runners API: ecs-light is carried by exactly three registrations (64c, hk-3, sg), matching the documented per-host convention, and every lane in REGISTERED_LANES (ecs-light/ecs-agent/ecs-qwen/ecs-win/qwen-benchmark-dsw-hk-eas) exists on real runners — so the label-before-workflow ordering the PR claims holds. HELPER_TESTS wiring includes the new guard, and the guard suite passes 44/44 against this tree.
On the three open Critical threads about parser completeness: I independently agree they are structurally valid. A runs-on value that is a pure expression (no literal self-hosted text) and a YAML-alias value both land in neither sets nor unreadable — silently skipped, not fail-closed; and the ${{ exemption is keyed on label text, so one expression label in a set exempts the whole set and an invented literal lane could ride along. None of these forms exist in the current workflows directory (the suite is green), so today's practical risk is low — but they contradict the file's own fail-closed header claim, so the guard is worth hardening (or the header softened to state the actual contract) before this test becomes the thing people trust to catch the next #10537. Not blocking the lane move on my side; leaving the completeness call to the author and the existing threads.
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed head 85d33d7. The eight lane changes themselves look clean: they only replace ecs-qwen with ecs-light while preserving the trust guards, MAINTAINER_ECS_RUNNER_DISABLED fallback, and hosted routing. The three live ecs-light runners are also online.
I am requesting changes because the new global guard still has two reproducible false-green paths:
- A valid runs-on list that combines an unregistered static lane with ${{ matrix.runner }} is skipped as soon as any label contains an expression. I added such a probe workflow against this head and runner-lane-labels.test.mjs still passed 44/44.
- ci.yml assembles ubuntu_runner inside pick_runner and four jobs consume that output. Changing both assembled ecs-qwen labels to ecs-invented also left the guard green at 44/44.
Those paths can still create the exact indefinite queue the guard claims to prevent. Please close the existing R2-4 and R2-5 threads with end-to-end regression tests, or keep this lane-routing PR narrow by splitting/removing the global guard and narrowing its claim. I view the broader YAML-spelling and test-wiring threads as hardening suggestions once the supported scope is explicit.
Separately, the current Qwen Code CI Test job hit its 90-minute timeout and the coverage follow-up failed because the artifact was unavailable; that appears unrelated to this diff, but the required run still needs a green rerun before merge.
Round-2 review follow-ups on #10575. The round-1 fix enumerated two unreadable spellings; review demonstrated six more families slipping through, plus two fail-open layers the spelling classifier never sees. Enumerating entrances cannot close a YAML grammar, so this closes each surface by shape instead: - Key recognition tolerates a quoted key and space before the colon; any block scalar, block sequence, alias, anchor or comment-led value is refused by its leading character; an unclosed flow sequence is refused as such; and any value naming self-hosted that produced no parsed set is refused as unrecognized. The refuse suite pins the whole block-marker family generated from one list, plus every demonstrated entrance. - The `${{` exemption is now keyed on resolvability, not label text: a label reading a real runtime context (matrix/needs/inputs/...) still skips, but a constant expression label and a static lane written beside a dynamic one fail closed. update-ecs-runner-qwen.yml's pure fan-out keeps its skip. - Candidate values now include whole-line VAR='["..."]' assignments, because ci.yml's pick_runner assembles the Linux runner set off the runs-on line and four jobs consume it through fromJSON; mutating that assignment to an invented lane previously shipped green. This is the one deliberate relaxation of the no-run-body rule, scoped to that exact shape; hosted-only assignments stay unflagged. - Per-file handling is extracted into a pure laneTestPlan the main loop maps to it() one-to-one, with the found-map accounting flowing through the plan. The wiring is now testable: synthetic cases assert the planned bodies actually throw for unreadable spellings, invented lanes, mixed dynamic sets and assignment-carried lanes. Verified by mutation, each against its own guard: the 9-entrance probe corpus goes red (9 failures) and greens on removal; narrowing the block shape drops the | family red; deleting the unreadable registration branch goes red; restoring the blanket dynamic skip goes red; removing the assignment scan goes red. 68/68 on the real corpus, with no live workflow reclassified.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:65 — [probe] RA1-2 PLATFORM_LABELS omits GitHub's auto-assigned arm/arm64/x86/macos labels (legit ARM/macOS lane jobs refused).github/scripts/ci/runner-lane-labels.test.mjs:260 — [probe] RA3-1 orphan-lane assertion has no deletion witness (removing the it block ships 67/67 green).github/scripts/ci/runner-lane-labels.test.mjs:345 — [probe] RA6-1 refuses/parses loop-generated it() registrations unwitnessed (deletion ships green)
Convergence: round 3 posted 7 inline comment(s), 7 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in round 2; 7 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 7 条行内评论,其中 7 条是首次提出;上一轮发布了 5 条(其中 5 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 2 轮已出过发现,本轮又有 7 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
12 Suggestion(s) were drafted inline past the resolved critical posting floor — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s); the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 4, not a blocker) — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:281 — [review] R4-3: The all-caps exemption exists for the contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]), …) author-association operand, but it is keyed on token shape, not on the co….github/scripts/ci/runner-lane-labels.test.mjs:373 — [review] R4-4: This prefilter re-implements checkSet 's hosted gate as a silent continue , dropping its "neither hosted nor self-hosted" failure branch — so a runner-set assembly naming….github/scripts/ci/runner-lane-labels.test.mjs:148 — [review] R4-5: The job-over-workflow env precedence implemented here (matching how GitHub evaluates ${{ env.* }} ) is correct but unpinned: the only env test defines LANE at workflow….github/scripts/ci/runner-lane-labels.test.mjs:401 — [review] R4-7: This title asserts "names a registered lane" for every accepted set, but the maintenance branch in checkSet deliberately pushes to oks WITHOUT lanes because ecs-upda….github/scripts/ci/runner-lane-labels.test.mjs:384 — [review] R4-8: The rewrite landed here changed the suite's test count, but the PR description's Reviewer Test Plan step 1 still tells reviewers to run node --test --test-concurrency=1 .g….github/scripts/ci/runner-lane-labels.test.mjs:648 — [review] R4-10: The tripwire counts problem-test and ok-test registrations into ONE counter, so deleting only the problems → assert.fail registration loop ships green — the oks regist….github/scripts/ci/runner-lane-labels.test.mjs:83 — [review] R4-11: The maintenance exemption matches the ecs-update- prefix SHAPE instead of a known label set, so any invented or typo'd ecs-update-* label passes checkSet in ANY workf….github/scripts/ci/runner-lane-labels.test.mjs:284 — [review] R4-6 (location 1 of 5): This fail-closed exit has no synthetic pin — one of five unpinned problem() branches across the walk (aggregate R4-6; the others: ~240, ~259, ~320, ~360….github/scripts/ci/runner-lane-labels.test.mjs:322 — [review] R4-6 (location 2 of 5): This fail-closed exit has no synthetic pin — one of five unpinned problem() branches across the walk (aggregate R4-6). For this one: a non-string, non-a….github/scripts/ci/runner-lane-labels.test.mjs:242 — [review] R4-6 (location 3 of 5): This fail-closed exit has no synthetic pin — one of five unpinned problem() branches across the walk (aggregate R4-6). For this one: a set containing tw….github/scripts/ci/runner-lane-labels.test.mjs:261 — [review] R4-6 (location 4 of 5): This fail-closed exit has no synthetic pin — one of five unpinned problem() branches across the walk (aggregate R4-6). For this one: an in-set expressio….github/scripts/ci/runner-lane-labels.test.mjs:362 — [review] R4-6 (location 5 of 5): This fail-closed exit has no synthetic pin — one of five unpinned problem() branches across the walk (aggregate R4-6). For this one: a job with neither …
Convergence: round 4 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 7 (7 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in rounds 2, 3; 1 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.)
中文说明
12 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论——发布下限因首次发现速率连续 2 轮未下降而提前生效;CLI 已将其移入下方延后清单(下限强制执行)。
收敛姿态下延后(第 4 轮,非阻断)——发布下限因首次发现速率连续 2 轮未下降而提前生效——已记录,本轮不要求修改:共 12 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 7 条(其中 7 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 2、3 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.3)
Review follow-ups on #10575: - The guard silently skipped any runs-on spelling its parser could not read — a block-sequence runs-on captures an empty value, an unquoted flow sequence yields no quoted tokens — so either spelling could ship an unregistered lane past a green suite, the exact incident class the guard exists to prevent. selfHostedLabelSets now returns those values as unreadable and the suite fails on each, naming the file and the spelling; a synthetic-text describe pins the contract for both spellings plus bare `runs-on: self-hosted` and block scalars. Verified end to end: a planted probe workflow with both invented-lane spellings turns the corpus scan red (2 failures), and removing the fail-closed block lets the same probe pass green again. - The eight workflow copies of the lane rationale still said "runner 1 on each ECS host", which the per-host split superseded — and eight copies of fleet topology is how they went stale in the first place. Each is now a one-line pointer to the registry header in runner-lane-labels.test.mjs, the single home of the convention. - review-config had no timeout-minutes; on a three-slot lane an uncapped job is a third of the lane if it wedges. Capped at 5 like its siblings.
Round-2 review follow-ups on #10575. The round-1 fix enumerated two unreadable spellings; review demonstrated six more families slipping through, plus two fail-open layers the spelling classifier never sees. Enumerating entrances cannot close a YAML grammar, so this closes each surface by shape instead: - Key recognition tolerates a quoted key and space before the colon; any block scalar, block sequence, alias, anchor or comment-led value is refused by its leading character; an unclosed flow sequence is refused as such; and any value naming self-hosted that produced no parsed set is refused as unrecognized. The refuse suite pins the whole block-marker family generated from one list, plus every demonstrated entrance. - The `${{` exemption is now keyed on resolvability, not label text: a label reading a real runtime context (matrix/needs/inputs/...) still skips, but a constant expression label and a static lane written beside a dynamic one fail closed. update-ecs-runner-qwen.yml's pure fan-out keeps its skip. - Candidate values now include whole-line VAR='["..."]' assignments, because ci.yml's pick_runner assembles the Linux runner set off the runs-on line and four jobs consume it through fromJSON; mutating that assignment to an invented lane previously shipped green. This is the one deliberate relaxation of the no-run-body rule, scoped to that exact shape; hosted-only assignments stay unflagged. - Per-file handling is extracted into a pure laneTestPlan the main loop maps to it() one-to-one, with the found-map accounting flowing through the plan. The wiring is now testable: synthetic cases assert the planned bodies actually throw for unreadable spellings, invented lanes, mixed dynamic sets and assignment-carried lanes. Verified by mutation, each against its own guard: the 9-entrance probe corpus goes red (9 failures) and greens on removal; narrowing the block shape drops the | family red; deleting the unreadable registration branch goes red; restoring the blanket dynamic skip goes red; removing the assignment scan goes red. 68/68 on the real corpus, with no live workflow reclassified.
8974f2a to
4333bab
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- DUP-1 tripwire counts registrations, not verdicts (mutant now executed end-to-end) — already recorded as R4-10 in the round-4 deferral list (review 5067140559)
- DUP-2 ecs-update-* maintenance exemption is an open prefix, not a set (two-arm probe witness) — already recorded as R4-11 in the round-4 deferral list (review 5067140559)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 5, not a blocker) — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:378 — [review] D5-1 bare-reference arm duplicates resolveExpression's matrix/env dispatch (drift trap).github/workflows/comment-attachment-guard.yml:63 (+3 locations) — [review] D5-2 three seconds-long twin jobs left on saturated ecs-qwen (+ web-shell-visuals-cleanup.yml:22, qwen-triage.yml publish-tmux:1932)
Convergence: round 5 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in round 4; 1 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already engage the critical posting floor — it resolved early, ahead of the round-6 schedule, because the first-time-finding rate has not fallen for consecutive rounds. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (1 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 5 轮,非阻断)——发布下限因首次发现速率连续 2 轮未下降而提前生效——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 5 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 4 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已处于 critical 发布下限——因首次发现速率连续多轮未下降,已先于第 6 轮的既定计划提前生效。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 1 条 Critical),首次发现的速率没有下降(本轮 1,上一轮 1),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Eight jobs that finish in seconds shared `ecs-qwen` with the 90-minute Test lane, so they queued behind it: a snapshot during the 2026-08-30 saturation had 121 jobs waiting, ~74% of them these. Measured over a day of runs they are 71% of the job count but 13% of the machine time, so the delay is head-of-line blocking, not capacity — moving them costs the CI lane almost nothing. They stay on the persistent pool. The comments above each `runs-on` say these jobs check out nothing and run no repository code, so the pool is safe and skips the saturated hosted queue; `ecs-light` keeps that intact while removing the queueing. Routing them back to `ubuntu-latest` would instead reintroduce the 2026-08-25 incident that ci-runner-routing.test.mjs already guards against, where a hosted backlog queued the autofix scan lane past its cron period. The fleet registers `ecs-light` on runner 1 of each of the five Linux hosts, giving the convention: 1 light, 2-15 agent, 16-25 qwen. The label was added and verified on the runners before this change. Out of scope, both deliberately: classify_pr publishes `ubuntu_runner` for every downstream Linux job and is asserted to agree with its own `pick_runner` step, so it needs its own design; autofix's route and review-scan carry the 2026-08-25 regression guard. Also adds runner-lane-labels.test.mjs, which fails when a workflow asks for a lane the registry does not list. Twice this month a workflow shipped a label no runner carried, and both times the symptom was silence rather than a red job -- GitHub queues such a job indefinitely and `timeout-minutes` does not count queue time. The test cannot prove a label is registered, but it makes inventing one a reviewable edit next to the note about registering it first.
Review follow-ups on #10575: - The guard silently skipped any runs-on spelling its parser could not read — a block-sequence runs-on captures an empty value, an unquoted flow sequence yields no quoted tokens — so either spelling could ship an unregistered lane past a green suite, the exact incident class the guard exists to prevent. selfHostedLabelSets now returns those values as unreadable and the suite fails on each, naming the file and the spelling; a synthetic-text describe pins the contract for both spellings plus bare `runs-on: self-hosted` and block scalars. Verified end to end: a planted probe workflow with both invented-lane spellings turns the corpus scan red (2 failures), and removing the fail-closed block lets the same probe pass green again. - The eight workflow copies of the lane rationale still said "runner 1 on each ECS host", which the per-host split superseded — and eight copies of fleet topology is how they went stale in the first place. Each is now a one-line pointer to the registry header in runner-lane-labels.test.mjs, the single home of the convention. - review-config had no timeout-minutes; on a three-slot lane an uncapped job is a third of the lane if it wedges. Capped at 5 like its siblings.
Round-2 review follow-ups on #10575. The round-1 fix enumerated two unreadable spellings; review demonstrated six more families slipping through, plus two fail-open layers the spelling classifier never sees. Enumerating entrances cannot close a YAML grammar, so this closes each surface by shape instead: - Key recognition tolerates a quoted key and space before the colon; any block scalar, block sequence, alias, anchor or comment-led value is refused by its leading character; an unclosed flow sequence is refused as such; and any value naming self-hosted that produced no parsed set is refused as unrecognized. The refuse suite pins the whole block-marker family generated from one list, plus every demonstrated entrance. - The `${{` exemption is now keyed on resolvability, not label text: a label reading a real runtime context (matrix/needs/inputs/...) still skips, but a constant expression label and a static lane written beside a dynamic one fail closed. update-ecs-runner-qwen.yml's pure fan-out keeps its skip. - Candidate values now include whole-line VAR='["..."]' assignments, because ci.yml's pick_runner assembles the Linux runner set off the runs-on line and four jobs consume it through fromJSON; mutating that assignment to an invented lane previously shipped green. This is the one deliberate relaxation of the no-run-body rule, scoped to that exact shape; hosted-only assignments stay unflagged. - Per-file handling is extracted into a pure laneTestPlan the main loop maps to it() one-to-one, with the found-map accounting flowing through the plan. The wiring is now testable: synthetic cases assert the planned bodies actually throw for unreadable spellings, invented lanes, mixed dynamic sets and assignment-carried lanes. Verified by mutation, each against its own guard: the 9-entrance probe corpus goes red (9 failures) and greens on removal; narrowing the block shape drops the | family red; deleting the unreadable registration branch goes red; restoring the blanket dynamic skip goes red; removing the assignment scan goes red. 68/68 on the real corpus, with no live workflow reclassified.
The registry header named the five current hosts, but hosts rotate — new hk machines are coming and two hosts are being released — and a host list in a comment is exactly the staleness R1-2 flagged. The convention that survives rotation is the shape: agent hosts carry ecs-agent everywhere, CI hosts carry ecs-qwen everywhere except runner 1, and runner 1 on each CI host is ecs-light so the light lane spans hosts. The runners API stays the only source of truth for which machines exist.
Round-3 review demonstrated what rounds one and two already suggested:
a hand-rolled line reader of runs-on spellings fails open on entrances
that cannot be enumerated shut — flow mappings, quoted keys, mixed
quoted/unquoted sequences, constants bound through env: and matrix:,
quoted literal arrays that YAML reads as one string label, and the
assignment shape drifting out from under its regex. Each round closed
the reported entrances and the next round found the class again.
So the guard now parses every workflow with the real `yaml` package —
the same root devDependency ci-runner-routing.test.mjs and six other
sibling workflow tests already import — and walks jobs.*.runs-on as
parsed values:
- Label arrays are read whatever the YAML spelling (block sequences,
flow, mixed quoting, comments), so an invented lane is CAUGHT rather
than skipped in every spelling the parser reads.
- `${{ matrix.* }}` and `${{ env.* }}` labels are resolved to their
in-file definitions and the resolved sets validated — GitHub
evaluates these, so the guard does too, instead of exempting them by
context name. update-ecs-runner-qwen.yml's ecs-update-* fan-out is
accepted through an explicit maintenance-label rule, not a skip.
- Whole-value expressions are judged by their embedded label arrays
(both arms of the routing conditionals); an expression with no
arrays must resolve in-file or it fails closed. All-caps token lists
(["OWNER","MEMBER","COLLABORATOR"]) are contains()-operands, not
runner sets, and are excluded by that shape — nothing the fleet
registers matches /^[A-Z_]+$/.
- A quoted literal array is refused as the string scalar GitHub reads
it as; bare unknown scalars, text-mixed expressions, unresolvable
references, and unparseable workflows all fail closed.
- Lane comparison is case-insensitive to match GitHub's label
matching; the pick_runner assignment scan stays, with a tripwire
pinning the live ci.yml lines to the scanned shape and a stated
price note for the run-body relaxation.
- The plan is pure ({problems, oks, lanes}); the main loop registers a
test per problem AND per validated set, and a counter tripwire fails
if that registration ever stops.
74 tests on the real corpus, no live workflow reclassified. Verified
by mutation, each against its own witness: blanket-skipping dynamic
labels (5 red), disabling the assignment scan (4), dropping the
quoted-literal refusal (2), deleting the registration loop (2),
dropping the exactly-one-lane assertion (3), dropping the case
normalization (2); restore returns 74/74.
Round-4 review: when a whole-value expression embedded a bracketed
array, only the arrays were judged — the quoted-scalar and bare-
reference fallback arms were returned past unexamined, so
`cond && fromJSON('[…]') || 'ecs-new-lane'` shipped green while GitHub
routes the fallback arm to a label no runner carries whenever the
condition is false. sdk-java.yml already routes with `|| 'ubuntu-
latest'` and `|| matrix.os` arms, so the shape is live, one word away
from the incident class.
The expression body is now split into its VALUE arms — top-level `||`
operands, taking the last top-level `&&` operand of each, which is
exactly what GitHub's `cond && value || fallback` can yield — and every
arm is judged: embedded arrays as before, quoted scalars through the
single-label rules (hosted arms stay accepted, unknown labels fail),
bare matrix./env. references resolved in-file, and anything else fails
closed. Condition operands are never judged, which keeps classify_pr's
live `!= 'true'` / `== 'merge_group'` comparison scalars out of
jurisdiction by construction rather than by exception.
Pinned both ways: invented scalar arm and invented matrix arm each
plan one failing test, the sdk-java hosted-scalar shape plans none; the
arms-unjudged mutant turns the pins red. 77 tests on the real corpus,
no live workflow reclassified.
Also rebased onto main, merging the HELPER_TESTS union (main added
ci-disk-pressure.test.mjs beside this branch's runner-lane-labels
entry).
Round-5 review demonstrated three escapes in the arm ladder — the all-caps exemption swallowing a fromJSON arm's array (vestigial since the arm split already excludes condition operands, and load-bearing for nothing), a quoted literal array arm judged as a label set when GitHub reads it as one string label, and a parenthesized (a || b) value whose alternatives were never re-split — and named the pattern: entrance enumeration keeps regenerating this family. Each value arm is now matched against a closed allowlist and anything else fails closed: one whole quoted scalar (judged first, as the string GitHub reads — the quoted-array arm is refused there), one whole fromJSON(...) call (every literal array inside judged, no exemptions; the `needs.… || '["ubuntu-latest"]'` consumer shape stays green because its indirection is checked at the pick_runner assembly site), or one bare matrix./env. reference resolved in-file. valueArms recurses after stripping parens, so both alternatives of a parenthesized value are judged; the residue test blanks quoted spans and call bodies so only top-level operators trigger the recursion, keeping the live consumer arms (|| inside call parens) whole. Pinned per entrance, each verified by mutation: restoring the all-caps exemption, letting the scalar rule pass '['-prefixed strings, and disabling the recursion each turn their pin red. 81 tests on the real corpus, no live workflow reclassified.
4333bab to
161d196
Compare
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- matrix/env name-resolution triplicated (ref, opRef, resolveExpression drift trap) — already recorded as D5-1 in the round-5 deferral list (review 5069369299) and re-confirmed as R7-8 in round 7 (review 5073874414)
- paren-strip corrupts doubly-wrapped expressions (fail-closed false red) — already recorded as R7-3 in the round-7 deferral list (review 5073874414)
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:656 — [probe] opaque-write exclusion regex re-derives the scanner's assembly shape (split-maintenance drift trap)
Convergence: round 9 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in round 6; 1 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (1 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 9 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 6 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 1 条 Critical),首次发现的速率没有下降(本轮 1,上一轮 1),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Round-9 review demonstrated seven shell write shapes the round-8
detector missed (export/declare reassignment, inline and appended
assignments, printf -v, a publish from a differently-named variable, a
literal echoed straight into $GITHUB_OUTPUT, a transformed expansion at
the publish site) and named the structural root: no value can reach a
needs-output consumer except through the line that writes <name>= into
$GITHUB_OUTPUT, so that write is the thing to verify — not an
enumeration of assignment spellings.
The consumer vouch now audits the named job's run text around that
anchor: the job must publish the output through exactly
echo "<name>=${<name>}" >> $GITHUB_OUTPUT (any other publish shape
fails closed), and after removing the scannable literals and that
accepted publish line, ANY remaining <name>= / += / printf -v text
fails closed as an unreadable write — shapes are rejected by residue,
not recognized by list. The live pick_runner wiring passes the audit
as-is (three scannable literals, one exactly-forwarding publish, no
other ubuntu_runner writes) and the accepting fixtures now carry the
publish line, matching the live shape.
Both prescribed pins added (differently-named publish variable, export
reassignment), each red under its reverting mutation (publish
attribution off, opaque residue check off). Regression-audited: the
real-corpus verdict diff against the previous commit is empty;
ci-runner-routing 23/23 and ci-platform-lanes 30/30 unchanged. 99
tests.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- literalShape re-derives the scanner's assembly shape (split-maintenance drift trap) — already recorded as D9-1 in the round-9 deferral list (review 5076346167)
- tripwire registeredEnforcementTests counts a hand-incremented lumped counter — already recorded as R4-10 in the round-4 deferral list (review 5067140559), re-confirmed as DUP-1 (round 5), V-3 (round 6), R7-6 (round 7)
Not explored to full depth (tool budget reached): "agent 4": none — nothing was cut short..
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:699 — [probe] acceptedPublish wider than the exact-shape contract (stray-brace shapes accepted; silent hosted drift).github/scripts/ci/runner-lane-labels.test.mjs:491 — [probe] the audit.accepted === 0 disjunct (no readable publish at all) is pinned by no test (mutant survives 99/99).github/scripts/ci/runner-lane-labels.test.mjs:516 — [probe] post-audit no-checkable-producer branch has no test (mutant survives 99/99).github/scripts/ci/runner-lane-labels.test.mjs:691 — [probe] stray CJK ideograph 残 corrupts the fail-closed contract comment (ANY残 remaining)
Convergence: round 10 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in round 6; 1 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (1 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未探索到全部深度(达到工具调用预算):"agent 4":none — nothing was cut short.。
收敛姿态下延后(第 10 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 10 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 6 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 1 条 Critical),首次发现的速率没有下降(本轮 1,上一轮 1),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /takeover from 10 |
|
🤝 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. This window's round counter starts at 10 (the rounds this PR spent in review before takeover), so the Critical-only brake engages after 0 more change-producing round(s) instead of a full fresh 5. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本窗口轮次计数从 10 起算(即本 PR 托管前已进行的评审轮数),因此再经过 0 个产生改动的轮次即进入 Critical-only,而非重新计满 5 轮。移除 |
Round-10 review demonstrated four entrances the round-9 publish-write
audit still missed, all at links the textual model never reads: writes
to the consumed variable carrying no <name>= text (read -r, mapfile,
for targets, indexed assignments, unset, sourced scripts), the job's
outputs: mapping — the hop GitHub actually serves — which was never
read at all, a rebound echo command word executing in the builtin's
place, and cross-step chains where a literal in one step vouches a
publish in another whose ${name} expands from an env binding.
The vouch is inverted from enumerating write shapes to a closed
allowlist over the whole chain: the job's outputs mapping must forward
the consumed name from exactly \${{ steps.<id>.outputs.<name> }}; that
one step must hold at least one scannable name='["…"]' literal and the
exact publish echo "name=${name}" >> "$GITHUB_OUTPUT"; nothing else in
the step may mention the name except pure $name/${name} reads, rebind
the echo command word, or source another script; and no
workflow/job/step env scope may bind the name. Every other shape fails
closed. The live pick_runner wiring passes unchanged (mapping, step id,
three literals, one exact publish, one pure read, no env binding).
Eleven new pins — the four demonstrated entrances plus one per chain
link (env binding, sourced script, no literal, no publish, missing job,
missing step, env-free cross-step) — each verified red under its
reverting mutation; eight of them also red against the pre-round guard.
Existing producer fixtures now carry the mapping and step id, matching
the live shape. Regression-audited: corpus verdicts identical across
all 52 workflows, zero reclassifications; ci-runner-routing 23/23
unchanged. 110 tests.
|
🤖 Addressed the latest review feedback (round 11/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 11/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #10575 (round 11, Critical-only window)Feedback triageCritical-only mode is active (window seeded at round 10 by takeover; round [rc:3905063558] [Critical] R6-1 (fix-induced) — consumer vouch models the step-write chain textually — RESOLVED IN CODEThe finding demonstrated four entrances by execution against the round-9 head,
Reproduced first: a probe harness built from the exact head code confirmed Fix — the trust model is inverted to a closed allowlist over the whole
Live wiring constraint honored: ci.yml's mapping ( Witnesses: eleven new synthetic pins — the four demonstrated entrances Mutation probes (remove/negate one closure, confirm the witness reds, Other feedback in the window
Changes
No conflict resolution this round ( Verification
中文说明Autofix 轮次总结 — PR #10575(第 11 轮,仅 Critical 窗口)反馈分类当前处于仅处理 Critical 的模式(窗口由 takeover 从第 10 轮起算,轮次计数已达 5)。确定性刹车已将所有非 Critical 反馈延后——这些条目保持开放、留待人工跟进,本轮按刹车规则未做任何改动。 [rc:3905063558] [Critical] R6-1(修复引入)— 消费者背书按文本建模 step 写入链 — 已在代码中解决该发现通过在第 9 轮 HEAD 上实际执行,演示了四个入口,全部位于文本模型读不到的环节:
先复现后修复:用 HEAD 原代码构建的探针确认四种形状全部返回 修复——按发现的处方把信任模型反转为整条链的封闭允许清单;对写入形状的枚举已移除:
现存接线约束已保住:ci.yml 的映射( 见证:新增 11 条合成 pin——四个已演示入口,外加每个链环各一条(仅 env 绑定、source 脚本、无可扫描字面量、无发布、缺失生产者 job、缺失被点名 step、无 env 的跨 step)。所有既有生产者用例迁移到共享的 变异探针(逐个移除/取反一条闭合、确认见证变红、再恢复):残差网、映射跳、job 存在、step 存在、env 绑定、字面量存在、发布存在、echo 重绑定、source 检查、按 step 作用域(重新汇总的突变体)、纯读取擦除(现存语料变红)——11 条闭合全部见证变红;恢复后套件全绿。对轮前守卫,11 条新 pin 中有 8 条为红(四个入口 + env 绑定、source、跨 step/映射两跳);其余 3 条钉住的是旧模型用其自身(已被替换的)逻辑碰巧也会产生的失败即报结果。 窗口内的其他反馈
变更
本轮无冲突处理( 验证
🦷 Gate advisory — this round resolves a Critical/Request-changes finding with test-only changes (machine-measured): the bite check cannot verify a test-side fix, so the resolution rests on the round summary alone. · 本轮以纯测试改动解决 Critical/Request-changes 反馈(门自动测量):bite 检查无法验证测试侧修复,该解决仅以轮次摘要为凭。 Deferred non-Critical feedbackCritical-only mode is active: the round counter reached 5 (this window was seeded at round 10 by
中文说明已进入仅处理 Critical 的模式:轮次计数已达 5(本窗口由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 1 selected review thread(s). · 已关闭全部选中的 1 条评审线程。 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/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- triplicated matrix./env. reference resolution (opRef / arm ladder / resolveExpression) — already recorded as D5-1 in the round-5 deferral list (review 5069369299), re-confirmed as R7-8 (round 7, review 5073874414) and in rounds 8-10
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 10.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
.github/workflows/ci.yml:436 — [review] D11-1 the guard's only CI invocations ride a job whose runs-on consumes the very routing the guard validates (a ci.yml-local routing break queues the guard itself and never fires red).github/scripts/ci/runner-lane-labels.test.mjs:699 — [probe] D11-2 hop-2's uses-step subcase is the only new fail-closed branch with no synthetic pin (narrowed-clause mutant ships green).github/scripts/ci/runner-lane-labels.test.mjs:853 — [probe] D11-3 the pick_runner tripwire's failure message misdiagnoses the one change it exists to catch (a new producer variable).github/scripts/ci/runner-lane-labels.test.mjs:1055 — [probe] D11-4 the double-quoted assignment pin certifies a spelling bash breaks at runtime and repels the escaped spelling that works.github/scripts/ci/runner-lane-labels.test.mjs:783 — [probe] D11-5 every uses: job is skipped unconditionally — a cross-repo reusable workflow's runs-on is vouched green unread
Convergence: round 11 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in round 6; 2 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 2, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:反向审计——在 10 轮的反审轮数上限内未收敛。
收敛姿态下延后(第 11 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 11 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 6 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 2 条 Critical),首次发现的速率没有下降(本轮 2,上一轮 1),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
Round-11 review confirmed the round-10 entrances closed but demonstrated twelve more shapes the textual shell modeling still vouched open (R6-1), and a bracketed label literal vouched as its quoted subset alone while unquoted content rode fromJSON to GitHub unread (R11-1). R6-1: the residual check is inverted from enumerating dangerous shapes to a closed allowlist — every producer line that is neither a scannable name='["…"]' literal nor the accepted publish must match a shape provably unable to write the name or run code the guard did not read (constant assignments, read-only case/if skeletons, a read-only echo of the name); anything else fails the vouch. Structural checks close the classes the per-line view cannot see: the step must resolve to bash, no visible env scope (step/job/workflow/container) may bind the name or a BASH_ENV/ENV/BASH_FUNC_* startup key, and no step of the job may write $GITHUB_ENV/$GITHUB_PATH. The earlier echo-rebind and source enumerations are deleted — the allowlist subsumes them — and a backslash continuation needs no check of its own: no vouched shape ends in one outside a comment, where bash ignores it. R11-1: bracketTokens reports the residue beside the quoted spans; a bracketed literal carrying anything but commas and whitespace fails closed at all four call sites instead of vouching the quoted subset. The live pick_runner chain vouches unchanged. 25 new pins — one per demonstrated entrance and residue shape — each verified red under its reverting mutation. 139 tests.
|
🤖 Addressed the latest review feedback (round 12/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 12/100 轮)。改动内容与我反驳保留之处如下: Address-review summary — PR #10575 (round 12)Critical-only mode is active; both actionable findings this round are Critical Feedback points and dispositions[rc:3908969100] R6-1 (Critical) — the
|
| Removed clause | Pins going red |
|---|---|
| residual allowlist loop | 14 (all residual entrances incl. backslash, plus the legacy rebind/source pins) |
| shell-must-be-bash check | 2 (step shell, workflow-default shell) |
BASH_ENV/BASH_FUNC_* key check |
2 (workflow scope, job scope) |
sibling $GITHUB_ENV/$GITHUB_PATH scan |
2 (both sibling pins) |
container.env scope |
1 (container env pin) |
| residue check forced true | 4 (all residue pins) |
Every probe was restored afterward; final run 139/139 green. The probes also
caught two real composition bugs before commit: the sibling pins originally
used if false; then (caught by the allowlist, not the sibling scan — the
dead branch now uses a vouched [[ ]] shape), and a dedicated backslash check
that was strictly redundant with the allowlist (dropped).
Verification
node --test --test-concurrency=1 .github/scripts/ci/runner-lane-labels.test.mjs— passed: 139/139 (baseline before the change: 114/114; this is the exact invocation ci.yml's HELPER_TESTS step uses)- Mutation probes: 6 clause removals, each turned its own pins red, all restored — final run 139/139 passed
npx prettier --check .github/scripts/ci/runner-lane-labels.test.mjs— clean after--writenpx eslint .github/scripts/ci/runner-lane-labels.test.mjs— passed (no findings)npm run build— passednpm run typecheck— passednpm run lint— passed- No settings source changed →
npm run generate:settings-schemanot required; no package code changed → no focused Vitest run applies (the touched behavior is exercised by the node:test suite above, not the bundled CLI)
中文说明
处理评审总结 — PR #10575(第 12 轮)
当前处于仅处理 Critical 的模式;本轮可执行的发现是两条 Critical 行内评论。两者均已在代码中解决,由新的合成钉扎作证,并做了变异探针验证。一个提交:31f1fc3a01(ci: close the producer vouch's residual surface structurally)。无需合并基线分支(--conflict false)。
反馈点及处置
[rc:3908969100] R6-1(Critical)— vouchProducer 闭环允许清单作为整体仍然敞开失败 → 已解决
先复现了该类失败:守卫用文本形状识别来建模生产者步骤的 shell 执行,凡是该模型没有点名的形状,都能递送一个守卫从未读过的值。修复按结构性方式关闭该类,而不是做第十二次枚举:
- 闭环残留允许清单——即发现所要求的反转。生产者步骤体中,凡既不是可扫描的
name='["…"]'字面量、也不是被接受的 publish 的行,都必须匹配一个被证明无法写入被消费变量、也无法执行守卫未读代码的形状:空行/注释行、常量右值赋值、只读的case/if [[ ]]控制骨架、以及对被消费名的只读 echo。任何不匹配任何形状的行都让背书失败即报。新的写入形状因此默认失败即报,而不是在被枚举之前一直敞开失败——仅这一条就关闭了入口 (a) 构造标识符、(b) env 喂送的eval、(c) 对$GITHUB_OUTPUT的第二次写入(含构造键与cat >>)、(d) 行中间的source/.、(g)enable -n echo、(i) 委托子 shell(bash ./x、./x、env bash)与 (j)trap/shopt/alias重绑。 - 解析后的 shell 必须是 bash(step → job defaults → workflow defaults)——关闭 (k) 非 bash 主体。
- env 作用域现在包含 container 的 env,并检查
BASH_ENV/ENV/BASH_FUNC_*启动键——关闭 (h) 启动注入与 (l)job.container.env绑定。 - 生产者 job 的任何步骤都不得写
$GITHUB_ENV/$GITHUB_PATH——关闭 (e) 兄弟步骤经 env 注入喂送死分支字面量,以及 (g) 中GITHUB_PATH垫片的那一半;这两者从生产者步骤本身不可见。 - 反斜杠续行 (f) 不需要专门检查:没有任何被背书的形状会在注释之外以续行反斜杠结尾,而 bash 会忽略注释内的反斜杠换行。变异探针证实该专项检查无见证/冗余,因此删除而不是带上它。
早前两个枚举式检查(echo 重绑、行首 source)被删除——允许清单已将其涵盖,它们原有的钉扎现在为允许清单作证。现存 pick_runner 链路的背书保持不变(已验证:全部既有强制测试保持绿色)。
见证:describe('laneTestPlan fail-closed behavior') 中新增 21 条钉扎,每个已演示入口一条(13 条表格驱动的残留入口 + 兄弟 $GITHUB_ENV/$GITHUB_PATH、反斜杠续行、BASH_ENV、BASH_FUNC_*、step 级与 workflow 默认的非 bash shell、container env),各自断言恰好一个匹配 /unreadable/ 的 problem。
[rc:3908969107] R11-1(Critical)— bracketTokens 静默丢弃未加引号的内容 → 已解决
已复现:一个在带引号标签旁夹带未加引号标量的括号标签字面量('["self-hosted", "linux", "x64", "ecs-qwen", 123]')会被按「恰好其带引号子集」背书——一个守卫从未完整读过的集合。bracketTokens 现在返回 { tokens, residueOk }:剥掉带引号片段与外层括号后,剩余部分若不是逗号与空白即失败。全部四个调用点在残留不干净时记录 problem(embeds unquoted content the guard cannot judge),既有的 tokens.length === 0 分支保留,self-hosted 预筛现在也会对含残留的 assembly 进行审判。现存生产者字面量只含逗号/空白残留——现存背书不变。
见证:新增 4 条钉扎——发现点名的生产者链分支与消费侧 fromJSON 字面量分支,外加每个新增分支各一条(env 操作数解析、独立 assembly 预筛)。
[rv:5083676054] 评审主体(CHANGES_REQUESTED,「部分审查——缺口已披露」)→ 本轮无代码动作
该评审状态由上述两条 Critical 承载(均已解决)。主体中的其余内容按其自身表述本轮不可执行:矩阵/env 三处重复引用解析一项是已记录在第 5 轮延后清单中的 Suggestion(非 Critical;在 Critical-only 模式下被延后),五条 D11-* 条目在收敛姿态下明确「已记录,本轮不要求修改」,残余风险建议(land-with-residual-risk)仅为建议——它描述的出口是 maintainer 的风险接受决定,而非再开一轮评审。
延后的非 Critical 反馈 / 失败检查
被延后的 PR 评论由 Critical-only 闸门排除——不触碰、不回复、不解决其线程。列出的失败检查是 review-pr FAILURE(即产生本反馈的评审 job)与两个 CANCELLED 的 job——属于运行级联,不是可以在本检出中复现的代码缺陷;推送后工作流的 CI 重跑才是验证闸门。
变异探针(提交前验证每条子句的见证)
| 移除的子句 | 变红的钉扎 |
|---|---|
| 残留允许清单循环 | 14 条(全部残留入口含反斜杠,外加既有的重绑/source 钉扎) |
| shell 必须为 bash 的检查 | 2 条(step shell、workflow 默认 shell) |
BASH_ENV/BASH_FUNC_* 键检查 |
2 条(workflow 作用域、job 作用域) |
兄弟步骤 $GITHUB_ENV/$GITHUB_PATH 扫描 |
2 条(两条兄弟钉扎) |
container.env 作用域 |
1 条(container env 钉扎) |
| 残留检查强制为 true | 4 条(全部残留钉扎) |
每次探针后均已还原;最终运行 139/139 全绿。探针还在提交前抓出两个真实的组合错误:兄弟钉扎最初使用 if false; then(由允许清单而非兄弟扫描捕获——死分支现在改用被背书的 [[ ]] 形状),以及一个与允许清单严格冗余的反斜杠专项检查(已删除)。
验证
node --test --test-concurrency=1 .github/scripts/ci/runner-lane-labels.test.mjs— 通过:139/139(改动前基线:114/114;这正是 ci.yml 的 HELPER_TESTS 步骤使用的调用方式)- 变异探针:6 次子句移除,每次都使对应钉扎变红,全部还原后 — 最终运行 139/139 通过
npx prettier --check .github/scripts/ci/runner-lane-labels.test.mjs—--write后干净npx eslint .github/scripts/ci/runner-lane-labels.test.mjs— 通过(无发现)npm run build— 通过npm run typecheck— 通过npm run lint— 通过- 未改动任何 settings 源 → 不需要
npm run generate:settings-schema;未改动任何包代码 → 无对应的聚焦 Vitest 运行(改动的行为由上述 node:test 套件执行,而非打包后的 CLI)
🦷 Gate advisory — this round resolves a Critical/Request-changes finding with test-only changes (machine-measured): the bite check cannot verify a test-side fix, so the resolution rests on the round summary alone. · 本轮以纯测试改动解决 Critical/Request-changes 反馈(门自动测量):bite 检查无法验证测试侧修复,该解决仅以轮次摘要为凭。
Deferred non-Critical feedback
Critical-only mode is active: the round counter reached 5 (this window was seeded at round 10 by @qwen-code /takeover from 10, plus 1 change-producing round(s) since). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)
中文说明
已进入仅处理 Critical 的模式:轮次计数已达 5(本窗口由 @qwen-code /takeover from 10 从第 10 轮起算,此后又完成 1 个产生改动的轮次)。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 @qwen-code /retry 可开启新的计数窗口。)
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
🧵 Resolved all 2 selected review thread(s). · 已关闭全部选中的 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
left a comment
There was a problem hiding this comment.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R12-A triplicated matrix./env. reference resolution (opRef / arm ladder / resolveExpression) — already recorded as D5-1 in the round-5 deferral list (review 5069369299), re-confirmed as R7-8 (round 7, review 5073874414) and rounds 8-11
- R12-B the guard's HELPER_TESTS membership pinned by nothing — already recorded as D6-2 in the round-6 deferral list (review 5071661168)
- R12-C registeredEnforcementTests tripwire counts registrations, not verdicts (neutered assert.fail survives) — already recorded as R4-10 in the round-4 deferral list (review 5067140559), re-confirmed as DUP-1 / V-3 / R7-6 in rounds 5-10
- R12-D the 'unreadable type' fail-closed branch unpinned — already recorded as the R4-6 aggregate (location 2) in the round-4 deferral list (review 5067140559), re-confirmed as R7-5 (round 7, review 5073874414)
- R12-E the double-quoted producer-assignment pin blesses a spelling bash breaks at runtime — already recorded as D11-4 in the round-11 deferral list (review 5083676054)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-code-pr-review.yml:228 — [probe] D12-1 the ecs-light timeout-cap invariant the diff states is enforced once and pinned by no test
Convergence: round 12 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in rounds 6, 11; 2 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 2, previous 2), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
本轮确认的 5 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 12 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 12 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 2 条(其中 2 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 6、11 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 2 条 Critical),首次发现的速率没有下降(本轮 2,上一轮 2),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
9 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- double-quoted producer-assignment pin blesses a spelling bash breaks at runtime (.github/scripts/ci/runner-lane-labels.test.mjs:106) — already recorded as D11-4 in the round-11 deferral list (review 5083676054), re-confirmed as R12-E in rou…
- PLATFORM_LABELS omits the auto-assigned arm/arm64/macos labels (.github/scripts/ci/runner-lane-labels.test.mjs:77) — already recorded as RA1-2 in the round-3 deferral list (review 5064059149), re-confirmed as RA-7 in round 7
- platform exclusivity covers only linux+windows, so the architecture mix opens once PLATFORM_LABELS expands (.github/scripts/ci/runner-lane-labels.test.mjs:361) — already recorded as RA-10 in the round-7 deferral list (review 5073874414)
- standalone-assembly self-hosted prefilter is unpinned and re-implements the hosted gate as a silent continue (.github/scripts/ci/runner-lane-labels.test.mjs:949) — already recorded as R7-7 in the round-7 deferral list (review 5073874414) an…
- MAINTENANCE_LABEL ecs-update- prefix exemption accepts any invented label (.github/scripts/ci/runner-lane-labels.test.mjs:83) — already recorded as R4-11 in the round-4 deferral list (review 5067140559)
- pick_runner tripwire message misdiagnoses a new producer variable (.github/scripts/ci/runner-lane-labels.test.mjs:1008) — already recorded as D11-3 in the round-11 deferral list (review 5083676054)
- matrix./env. reference resolution implemented three times (.github/scripts/ci/runner-lane-labels.test.mjs:553) — already recorded as D5-1 in the round-5 deferral list (review 5069369299), re-confirmed as R7-8 and R12-A
- ecs-light timeout-cap invariant enforced once and pinned by no test (.github/workflows/qwen-code-pr-review.yml:228) — already recorded as D12-1 in the round-12 deferral list (review 5085923393)
- valueArms paren-strip mangles doubly-wrapped expressions (.github/scripts/ci/runner-lane-labels.test.mjs:185) — already recorded as R7-3 in the round-7 deferral list (review 5073874414)
Not explored to full depth (tool budget reached): "agent 6c": ESLint over .github/scripts/ci/runner-lane-labels.test.mjs (ci.yml:524 node scripts/lint.js --eslint , config block eslint.config.js:464 ) — no node_module….
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 13, not a blocker) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:254 — [review] Residual allowlist has no shape for inert honest shell lines.github/scripts/ci/runner-lane-labels.test.mjs:361 — [review] A Windows platform set carrying a Linux-only lane is vouched.github/scripts/ci/runner-lane-labels.test.mjs:575 — [review] The unquoted-residue rule is written three times and the….github/scripts/ci/runner-lane-labels.test.mjs:853 — [probe] The sibling $GITHUB_ENV scan is order-blind and its…
Convergence: round 13 posted 4 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in rounds 6, 12; 2 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (4 Critical(s)), the rate of first-time findings is not falling (this round 2, previous 2), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 9 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未探索到全部深度(达到工具调用预算):"agent 6c":ESLint over .github/scripts/ci/runner-lane-labels.test.mjs (ci.yml:524 node scripts/lint.js --eslint , config block eslint.config.js:464 ) — no node_module…。
未审查:反向审计——评审时间预算不足,未能开始第 6 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 13 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 13 轮发布了 4 条行内评论,其中 2 条是首次提出;上一轮发布了 2 条(其中 2 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 6、12 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 4 条 Critical),首次发现的速率没有下降(本轮 2,上一轮 2),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
R13-1: REGISTERED_LANES never learned ecs-qwen-hk4-host, which main's release.yml has routed eight jobs to since the release-validation isolation change landed. The guard scans all 52 workflows rather than the diff, so the merged tree failed eight assertions — the required Test check red on this PR and on every PR after it merges, over a lane question unrelated to them. The registry is fixed rather than release.yml, whose runs-on string the release-workflow suite pins verbatim for all eight jobs. The label demonstrably exists on the fleet: actionlint.yaml lists it, added by the same commit that began routing to it, and release.yml documents the deliberate host-wide pin. R13-2: bracketTokens judged payloads with a hand-rolled lexer that erased every bracket before the residue test and paired any quote with any quote, so a nested or unbalanced array and a single-quoted or mismatched payload were each vouched as the flat set the lexer read out of the text — while fromJSON IS JSON.parse, which throws on two of those and returns a nested value for the other two. The lexer is gone: the payload goes to the authority being modelled, and only a flat array of JSON strings vouches. The failure path keeps a double-quote scan for the message alone, never for a vouch, so both existing diagnostics stay accurate. R12-1: valueArms took only the last && operand of every || part. In the terminal part — and in an expression with no || at all — every && operand can BE the value, because GitHub's `a && b` yields `a` itself when `a` is falsy and there is no following || to swallow it. An empty string was certified on the expression path while the scalar path already rejected it. Earlier parts keep discarding their condition operands, which is what keeps the accepted routing green, and a parenthesized group inherits the terminality of the part it sits in. R6-1: four seams in the producer vouch. A `uses:` sibling is opaque code that can plant the consumed name into $GITHUB_ENV for the publish to expand; a sibling body can build the sink's name so the literal spelling the scan looks for never appears; a container binds environment through docker-create options and image ENV that this file cannot read at all; and an assignment targeting GITHUB_OUTPUT rebinds where the accepted publish lands, leaving the real output file empty so every consumer silently takes its hosted fallback. Refusing the container makes the container.env scope unreachable, so that entry came out of the enumeration instead of being extended by two more unreadable scopes. The target lookahead is scoped to the assignment position, so the publish's own read of that variable is untouched. The sibling scan's substring posture is named as inherently incomplete in its comment; the two closures that do not depend on recognizing a spelling are refusing an opaque step and refusing a container. The live pick_runner chain vouches unchanged. 13 new pins — one per entrance the findings name, plus one witnessing that terminality is propagated rather than assumed — each verified red under its reverting mutation, nine mutations in all. 152 tests.
|
🤖 Addressed the latest review feedback (round 13/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 13/100 轮)。改动内容与我反驳保留之处如下: Round 14 — address-review summary (PR #10575)Critical-only mode was active (window seeded at round 10 by Headline: the red required Feedback points and decisions
|
| payload | guard at HEAD | JSON.parse of the same text |
|---|---|---|
["self-hosted", ["linux","x64","ecs-qwen"]] |
problems=0 lanes=["ecs-qwen"] |
OK → nested, not the certified flat set |
[["self-hosted","linux","x64","ecs-qwen"]] |
problems=0 lanes=["ecs-qwen"] |
OK → nested, not the certified flat set |
["self-hosted","linux","x64","ecs-qwen"]] |
problems=0 lanes=["ecs-qwen"] |
THROWS (position 43) |
['self-hosted','linux','x64','ecs-light'] |
problems=0 lanes=["ecs-light"] |
THROWS (unexpected ') |
["self-hosted","linux","x64",'ecs-qwen"] |
problems=0 lanes=["ecs-qwen"] |
THROWS (unexpected ') |
flat control ["self-hosted","linux","x64","ecs-qwen"] |
problems=0 lanes=["ecs-qwen"] |
OK → the flat set ✓ |
The nested and unbalanced spellings were vouched through all four entrances (the fromJSON literal arm, the resolved env. operand, the matrix./env. operand path, and the standalone judgeAssembly scan), each measured at problems=0.
Change. Took the finding's root fix — stop lexing, parse. bracketTokens now hands the payload to JSON.parse behind a try and vouches only a flat array of JSON strings; everything else returns parseOk: false and the callers fail closed as they already did. residueOk → parseOk at its four call sites, since the residue concept is gone.
This is the subtractive option the diff-growth guidance asks for: it removes the hand-rolled token regex and the bracket-erasing residue rule instead of adding another guard, and it closes the quote-delimiter half and the bracket-structure half with one change.
The failure path keeps a double-quoted span scan used for the diagnostic message alone — never for a vouch — so the two existing messages stay accurate and no existing pin's expectation moved. One message was reworded (has no quoted labels → has no JSON-quoted labels) because the delimiter class narrowed to JSON's own, which made the old wording inaccurate for a newly-reachable shape. No pin asserted it.
Constraints held. The flat spellings the finding names all stay green: the pick_runner assignment pin, the accepted table, the live-chain tripwire it('the pick_runner assembly in ci.yml still matches the scanned shape'), and the empty-quoted-label case the *-not-+ quantifier existed for (RA-3 — JSON.parse keeps "" as an element, so it still surfaces and still fails the exactly-one-lane rule). JSON.parse('[]') still yields an empty token set and still fails the empty-label-set rule. The slice-the-outer-brackets approach the finding warns against was not used.
Swept for the single-quoted-inside literal the finding says the parse route would newly fail: none exists in any of the 52 workflows, and the whole-fleet problem count went 8 → 0.
[rc:3911019607] + [rc:3913098487] R12-1 — valueArms judges only the last && operand — Required — FIXED (both threads)
Reproduced on the unmodified guard:
| probe | guard at HEAD |
|---|---|
P1 needs.preflight.outputs.go && fromJSON('[…ecs-light]'), no ` |
|
P2 ${{ "" && fromJSON('["ubuntu-latest"]') }} |
problems=0 |
P3 bare '' scalar (comparator) |
problems=1 — "routes to no known hosted image" |
| P4 with-fallback control | problems=0 lanes=["ecs-qwen"] |
P2 vs P3 is the platform-independent half of the claim and it reproduces exactly: the same empty value is rejected on the scalar path and certified on the expression path.
Change. valueArms(body, terminal = true). The arm loop now emits every && operand as a value arm for the LAST || part only — each falsy prefix returns itself as the value, and there is no following || to swallow it — and propagates that terminality through the paren-strip recursion, so a parenthesized group inherits the terminality of the part it sits in. Earlier parts keep last-operand-only.
After the fix: P1 → problems=1 (the prefix operand is none of the judgeable shapes, fails closed), P2 → problems=1 with a message byte-identical to the P3 scalar path, P3 and P4 unchanged.
Constraints held. Both accepted condition-operand pins stay green — accepts the conditional expression ci.yml routes with and accepts the hosted scalar fallback arm sdk-java routes with — and all 52 real workflows stay at 0 problems, because every expression this PR ships carries a fallback, so their condition operands sit in non-terminal parts. The doc comment that the code contradicted ("For each || part every operand that can BE the value is judged") was replaced with the rule the code now actually implements, including why non-terminal parts differ.
[rc:3911019598] + [rc:3913098492] R6-1 — vouchProducer's closed chain fails open at four seams — Required — FIXED (both threads, all four seams)
Reproduced all four at HEAD, each beside a control that behaved correctly:
| seam | guard at HEAD | control |
|---|---|---|
1a uses: sibling (actions/github-script@v7) / 1b local action |
problems=0 lanes=["ecs-qwen"] |
run: sibling writing $GITHUB_ENV → problems=1 |
2 constructed-name sibling (v='GITHUB'; v+=_ENV; … >> "${!v}") |
problems=0 |
same shape in the producer body → problems=1 |
3a container.options: '--env ubuntu_runner=…' / 3b --env BASH_ENV=… |
problems=0 lanes=["ecs-qwen"] |
container.env binds the name → problems=1; job.env binds it → problems=1 |
4 producer body GITHUB_OUTPUT=/tmp/redirect, as an assignment and as a case arm |
problems=0 (vouched) |
eval "$PAYLOAD" → problems=1; trusted_author=false → 0; MYPATH=x → 0 |
Changes — four clauses:
- Refuse any sibling step carrying
uses:— opaque action code this file cannot read at all. This is one of the two closures the finding calls durable, because it does not depend on recognizing a spelling. - Extend the sibling scan with the dereference primitives a constructed name requires (
${!,eval,declare). As the finding asked, the comment names the incompleteness of the substring posture against deliberate obfuscation and points at the two closures that do not depend on it. - Refuse the producer job when it has a
container:at all —docker createoptions and image-definedENVcannot be read from the file. This also letjob?.container?.envcome out of the env-scope list: the new check returns first, so that entry was unreachable, and leaving it would have been a dead scope. Net effect is a broader closure for one line less enumeration. - A negative lookahead on the assignment target position, shared by
ASSIGN_SHAPEandCASE_ARM_SHAPE, refusingGITHUB_OUTPUT=. Scoped to the target, so the accepted publish (>> "${GITHUB_OUTPUT}", a read) is untouched — this is deliberately not the widening of the job-wide sibling scan the finding measured as refusing the four real consumers.GITHUB_ENV=/GITHUB_PATH=targets need no entry because the job-wide scan already refuses any body that mentions them. The lookahead is backtracking-sound:\s*can only consume whitespace, so the name match cannot slide past the first character to evade it (mutation-probed both ways, M5/M6).
Constraints held. The live producer vouch survives: ci.yml still yields problems=0 lanes=["ecs-qwen","ecs-win"]. Verified in-repo that all four clauses cost it nothing — classify_pr has exactly two steps, both run: steps, no uses:, no container:, no defaults:, and neither body contains eval/declare/${!. The existing container.env pin still passes; trusted_author=false and MYPATH=x stay vouched; the whole fleet stays at 0 problems.
Not implemented, recorded for a maintainer: the finding's durable closure — "stop modelling the shell at all", accept a producer whose right-hand side is a ${ENV_NAME} reference resolving through envValue to a bracketed literal in an env: scope, and move ci.yml's two label arrays into env:. See Follow-ups.
Follow-ups recorded (not silently dropped)
Per AGENTS.md's round-balloon rule (past ~5 rounds, land only Critical fixes and defer the rest with a record), two suggestions inside this round's Criticals are not implemented:
- Stop modelling the shell in
vouchProducer; move ci.yml's label arrays intoenv:. This is the review's own recommended durable closure for a family that has regenerated a new seam for eleven consecutive rounds, and it is the change most likely to end the loop rather than extend it. It was not undertaken unilaterally because it is not a mechanical fix: it replaces the bashcase/ifdecision inpick_runner(overAUTHOR_ASSOCIATION,EVENT_NAME,ECS_DISABLED,SAME_REPO,DISPATCH_LINUX_RUNNER) with GitHub-expression logic, which is a risk decision about how live CI routing picks runners, not a guard hardening. Maintainer call. This round instead closed all four measured seams and pinned each one. - Derive
REGISTERED_LANESfrom.github/actionlint.yaml. R13-1 is exactly the drift this would prevent — the registry was written on the branch and never learned a label main added, one merge wide. Declined this round for the reason above (it asserts runner facts in a second file); worth a follow-up if a maintainer accepts that trade.
Both review bodies' convergence and residual-risk sections recommend land-with-residual-risk — that the exit is a maintainer risk-acceptance decision rather than another review round. That recommendation is addressed to the maintainer and this round does not decide it.
Failed checks
- Test (ubuntu-latest Node 22.x): FAILURE — root cause R13-1. Both HELPER_TESTS steps execute the guard with no
continue-on-error:ci.yml:436(GitHub CI helper checks, on thegithub_ci_onlyprofile this.github-only PR selects) andci.yml:597(Run .github/scripts helper tests, onfull). Fixed, and verified locally with the exact CI command: 658 tests / 658 pass / 0 fail, exit 0 (8 failures at HEAD). - Post Coverage Comment (ubuntu-latest 22.x): FAILURE — collateral of the same root cause, diagnosed from the workflow rather than assumed:
post_coverage_comment(ci.yml:1261) declaresneeds: [classify_pr, test]and its first substantive step downloads the artifactcoverage-reports-22.x-ubuntu-latest, which thetestjob uploads atci.yml:749-753— far after the helper-check step atci.yml:428-436that failed. The artifact was therefore never produced and the download failed. It runs none of the guard's code. No separate defect; expected to clear withTest.
Changes
One file, inside this PR's own footprint: .github/scripts/ci/runner-lane-labels.test.mjs (+284 / −64, all test lines; well inside the 400-line test budget for this window).
Committed additively as e890052423 — ci: register the hk4 lane and close four guard fail-open seams — on top of origin/ci/short-job-lane, matching the subject style of this branch's two previous autofix-round commits. No amend, rebase, reset or history rewrite. git status is clean afterwards; the working tree contains no scratch or probe artifacts (all probe and mutation harnesses were written under /tmp, and the one scratch copy the mutation probes needed inside .github/scripts/ci/ was deleted and its deletion verified).
One environment note: this checkout had no git identity configured, so the commit could not be created. A repo-local (not --global) identity was set to qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> — the same identity this branch's two previous autofix-round commits (31f1fc3a01, 90f66378c6) were authored under. Nothing else about the repository configuration was changed.
No workflow, no CI or verification machinery, no .husky/, no .qwen/, no repo scripts/, no package source, no lockfile, no workspace manifest field, and no settings source — so npm run generate:settings-schema was correctly not needed.
--conflict false, so no merge of origin/main was performed. Branch left as checked out.
Verification
Every command below was actually run; nothing is asserted from reading the diff.
node --test --test-concurrency=1 .github/scripts/ci/runner-lane-labels.test.mjs— 152 tests, 152 pass, 0 fail (was 139/131/8 at HEAD)node --test --test-concurrency=1 $HELPER_TESTS(the exact commandci.yml:436andci.yml:597run, list read fromci.yml:82) — 658 tests, 658 pass, 0 fail, exit 0npm run build— passed (exit 0)npm run typecheck— passed (exit 0)npm run lint— passed (exit 0)npx prettier --check .github/scripts/ci/runner-lane-labels.test.mjs— clean. HEAD's copy was clean too; two of this round's additions needed formatting and were formatted. (scripts/lint.js --prettierrunsprettier --write ., not a check, so this was verified separately — and that repo-wide write reformatted 37 unrelated files, all of which were restored; the committed tree contains only the one intended file.)- Integration tests after
npm run bundle— not run: nothing here is exercised through the bundled CLI or the integration harness. The change is confined to a.github/scriptsnode:testguard. npx vitest run scripts/tests/release-workflow.test.js— 45 tests, 45 passed. Run to confirm the R13-1 decision to fix the registry rather thanrelease.yml:ecsRunsOnat:1563pins that exact runs-on string and the loop at:1579asserts it verbatim for all eight job names, so re-routing would have red this suite. It is green, i.e. the routing is untouched and still pinned.- Focused Vitest for
packages/*— not applicable: no package underpackages/changed, and the touched suite isnode:test, not Vitest.
Mutation probes — every clause this round adds has its own witness
Each clause was removed or negated on a scratch copy, the suite re-run, and the source restored byte-identical afterwards (verified programmatically; the scratch copy was deleted). Parent-suite rollup lines are excluded from the counts, since node's TAP reporter marks a describe red when any child fails.
| # | clause removed / negated | expected reds | measured |
|---|---|---|---|
| M1 | ecs-qwen-hk4-host registry entry |
8 release.yml subtests |
8 ✓ |
| M2 | JSON.parse authority → the old lexer (keeping the parseOk name) |
5 R13-2 pins | 5 ✓ |
| M3 | terminal ` | part emits every&&` operand → last only |
|
| M4 | terminality propagation into the paren recursion | 1 pin | 1 ✓ |
| M5 | CONTROL_TARGET lookahead in ASSIGN_SHAPE |
1 pin | 1 ✓ |
| M6 | CONTROL_TARGET lookahead in CASE_ARM_SHAPE |
1 pin | 1 ✓ |
| M7 | refuse a container: on the producer job |
2 container pins | 2 ✓ |
| M8 | refuse an opaque uses: sibling |
1 pin | 1 ✓ |
| M9 | sibling dereference primitives (${! / eval / declare) |
1 pin | 1 ✓ |
All nine witnessed: each mutation reds exactly its own pins and nothing else, and the unmutated baseline is 152/152.
Pins added this round (13)
R13-2 (5): nested fromJSON literal · unbalanced fromJSON literal · nested env. operand · standalone nested assembly · two non-JSON delimiter payloads (single-quoted and mismatched pair).
R12-1 (3): empty-string prefix in a terminal && chain · unjudgeable prefix in a terminal && chain · a non-terminal paren group keeps its condition operands unjudged (witnesses M4 specifically — it is green only because terminality is propagated).
R6-1 (5): opaque uses: sibling · sibling that builds the sink name by dereference · producer job running in a container · plus two residualEntrances table rows for the GITHUB_OUTPUT rebind, as an assignment and as a case arm.
Every one of these yielded problems=0 at HEAD, so each reds if its fix is removed.
中文说明
第 14 轮 —— 评审处理小结(PR #10575)
本轮处于「仅处理 Critical」模式(计数窗口由 @qwen-code /takeover from 10 从第 10 轮起算)。可执行区域中列出的四条 Critical 发现,全部先在受审 HEAD 上实际复现,再动手修改,并按根因修复。未触碰任何被延后的非 Critical 条目,也未改动本 PR 自身足迹之外的任何代码。
结论要点:变红的必需检查 Test 已恢复;三条「敞开失败」的 Critical 已做结构性关闭,而不是逐个入口封堵 —— 其中 JSON 分词那一条是靠删掉手写分词器、改用它所建模的权威来关闭的。
各条反馈与处置
[rc:3913098451] R13-1 —— REGISTERED_LANES 漏登 ecs-qwen-hk4-host —— Required —— 已修复
已复现。 在未改动的 HEAD 上执行 node --test --test-concurrency=1 .github/scripts/ci/runner-lane-labels.test.mjs,结果为 139 项 / 131 通过 / 8 失败 —— 恰好就是该发现点名的八条 release.yml: jobs.{prepare,quality_static,quality_build,quality_typecheck,workspace_tests,quality_scripts,integration_none,integration_docker}.runs-on → ["self-hosted", "linux", "x64", "ecs-qwen-hk4-host"] names an unregistered lane 子测试。该修复所依赖的两个前提都在仓库内核实过,而不是照搬结论:.github/actionlint.yaml:4 已经列出了这个标签(由 main 上开始路由到它的同一个提交加入),release.yml:53-58 则写明了这是有意的主机级钉定、以及它为何故意不携带 ecs-qwen。
改动。 只加了一条注册表条目,并按本文件既有的逐条目说明惯例补上了理由注释。
结果。 既有套件 139/139(加上本轮新钉扎后为 152/152);it('every registered lane is still referenced by a workflow') 保持绿色,因为 release.yml 的八个 job 提供了引用 —— 新条目不会被判为死条目。
修的是注册表而不是 release.yml:scripts/tests/release-workflow.test.js:1563 钉住了那段完全相同的 runs-on 字符串,:1579 对八个 job 名逐一断言,改路由会让该套件变红。
已拒绝并记录理由(未实施): 建议中「更持久的形式」—— 从 .github/actionlint.yaml 派生 REGISTERED_LANES。那需要把 ecs-light、ecs-agent、qwen-benchmark-dsw-hk-eas 补进该文件,也就是在第二份基础设施元数据文件里断言三个标签确实存在于 runner 上 —— 而这正是本仓库明确无法证明的事(守卫自己的头部注释:「在这里登记的注册表无法证明标签已存在 —— 那属于 runner 配置,不在本仓库里」)。在仅处理 Critical 的一轮里,这比所报缺陷的范围更宽,而且它把一份手工清单换成了另一处耦合,其正确性仍然依赖仓库之外的事实。它针对的漂移风险是真实的,已记入下方「后续跟进」。
[rc:3913098480] R13-2 —— bracketTokens 是没有 JSON 概念的分词器 —— Required —— 已按根因修复
已复现,在未改动的守卫上、覆盖该发现点名的每个入口,并以 JSON.parse 作为权威对照:
| 载荷 | HEAD 上的守卫 | 对同一文本调用 JSON.parse |
|---|---|---|
["self-hosted", ["linux","x64","ecs-qwen"]] |
problems=0 lanes=["ecs-qwen"] |
解析成功 → 嵌套值,不是被认证的那个扁平集合 |
[["self-hosted","linux","x64","ecs-qwen"]] |
problems=0 lanes=["ecs-qwen"] |
解析成功 → 嵌套值,不是被认证的那个扁平集合 |
["self-hosted","linux","x64","ecs-qwen"]] |
problems=0 lanes=["ecs-qwen"] |
抛错(位置 43) |
['self-hosted','linux','x64','ecs-light'] |
problems=0 lanes=["ecs-light"] |
抛错(意外的 ') |
["self-hosted","linux","x64",'ecs-qwen"] |
problems=0 lanes=["ecs-qwen"] |
抛错(意外的 ') |
扁平对照组 ["self-hosted","linux","x64","ecs-qwen"] |
problems=0 lanes=["ecs-qwen"] |
解析成功 → 正是那个扁平集合 ✓ |
嵌套与括号不配平的写法在全部四个入口都被背书(fromJSON 字面量分支、解析后的 env. 操作数、matrix./env. 操作数路径、以及独立的 judgeAssembly 扫描),每处实测均为 problems=0。
改动。 采用了该发现的根本修复 —— 不再分词,改为解析。bracketTokens 现在用 try 包住 JSON.parse,只对「字符串组成的扁平 JSON 数组」出具背书;其余一律返回 parseOk: false,由调用方按既有逻辑失败即报。四处调用点的 residueOk 改名为 parseOk,因为「残差」这个概念已经不存在了。
这正是 diff 增长指引所要求的做减法的选项:它删掉了手写分词正则与抹除括号的残差规则,而不是再加一道守卫,并且用一次改动同时关闭了引号分隔符与括号结构两半。
失败路径保留了一个只认双引号的扫描,仅用于生成诊断消息、绝不用于背书 —— 因此两条既有消息依然准确,也没有任何既有钉扎的期望被改动。其中一条消息改了措辞(has no quoted labels → has no JSON-quoted labels),因为分隔符类收窄为 JSON 自己的双引号后,旧措辞对一个新增可达的写法不再准确。没有钉扎断言过该消息。
约束已守住。 该发现点名的扁平写法全部保持绿色:pick_runner 赋值钉扎、已接受表、活链路 tripwire it('the pick_runner assembly in ci.yml still matches the scanned shape'),以及 * 而非 + 量词所服务的空引号标签情形(RA-3 —— JSON.parse 会把 "" 保留为一个元素,所以它仍然浮现,仍然让「恰好一个 lane」规则失败)。JSON.parse('[]') 仍然得到空的 token 集合,仍然让「空标签集合」规则失败。发现中警告的「把外层括号切掉」的做法没有被采用。
已按发现要求扫描内层单引号字面量(解析路线会新失败的那种写法):52 个 workflow 中一个都没有;全机群的问题数由 8 → 0。
[rc:3911019607] + [rc:3913098487] R12-1 —— valueArms 只审判最后一个 && 操作数 —— Required —— 已修复(两条线程)
已复现,在未改动的守卫上:
| 探针 | HEAD 上的守卫 |
|---|---|
P1 needs.preflight.outputs.go && fromJSON('[…ecs-light]'),无 ` |
|
P2 ${{ "" && fromJSON('["ubuntu-latest"]') }} |
problems=0 |
P3 裸 '' 标量(对照组) |
problems=1 —— "routes to no known hosted image" |
| P4 带兜底的对照形状 | problems=0 lanes=["ecs-qwen"] |
P2 与 P3 的对比是其中不依赖平台语义的那一半,且原样复现:同一个空值,标量路径拒绝,表达式路径放行。
改动。 valueArms(body, terminal = true)。分支循环现在只对最后一个 || 分支把每个 && 操作数都作为值分支输出 —— 每个假值前缀都会以自身作为值返回,而其后再没有 || 来吞掉它 —— 并把该终止性沿去括号递归传递下去,因此括号组会继承它所处分支的终止性。较早的分支保持只取最后一个操作数。
修复后: P1 → problems=1(前缀操作数不属于任何可审判形状,失败即报);P2 → problems=1,消息与 P3 标量路径逐字节相同;P3、P4 不变。
约束已守住。 两条已接受的条件操作数钉扎保持绿色 —— accepts the conditional expression ci.yml routes with 与 accepts the hosted scalar fallback arm sdk-java routes with;52 个真实 workflow 全部保持 0 个问题,因为本 PR 提交的每个表达式都带兜底,其条件操作数都位于非终止分支。那段与代码相矛盾的文档注释(「每个 || 分支中所有可能成为值的操作数都会被审判」)已替换为代码现在真正实现的规则,并写明了非终止分支为何不同。
[rc:3911019598] + [rc:3913098492] R6-1 —— vouchProducer 闭环链在四个接缝处敞开失败 —— Required —— 已修复(两条线程、全部四个接缝)
四个接缝全部在 HEAD 上复现,每个都配了一个行为正确的对照组:
| 接缝 | HEAD 上的守卫 | 对照组 |
|---|---|---|
1a uses: 兄弟(actions/github-script@v7)/ 1b 本地 action |
problems=0 lanes=["ecs-qwen"] |
写 $GITHUB_ENV 的 run: 兄弟 → problems=1 |
2 构造名字的兄弟体(v='GITHUB'; v+=_ENV; … >> "${!v}") |
problems=0 |
同一形状放在 producer 体内 → problems=1 |
3a container.options: '--env ubuntu_runner=…' / 3b --env BASH_ENV=… |
problems=0 lanes=["ecs-qwen"] |
container.env 绑定该名字 → problems=1;job.env 绑定 → problems=1 |
4 producer 体内 GITHUB_OUTPUT=/tmp/redirect,赋值形式与 case 分支形式 |
problems=0(被背书) |
eval "$PAYLOAD" → problems=1;trusted_author=false → 0;MYPATH=x → 0 |
改动 —— 四条子句:
- 对任何带
uses:的兄弟步骤失败即报 —— 本文件根本无法读取的不透明 action 代码。这是发现称为「持久」的两处关闭之一,因为它不依赖识别某种拼写。 - 给兄弟扫描补上构造名字所需的解引用原语(
${!、eval、declare)。按发现的要求,注释里明确写出了子串策略面对刻意混淆时天然不完备,并指向两处不依赖它的关闭。 - producer job 只要带
container:就整体失败即报 ——docker create选项与镜像定义的ENV无法从文件读取。这同时让job?.container?.env从 env 作用域清单中移除:新检查先返回,那一条已不可达,留着就是一个死作用域。净效果是用更少的枚举换来了更宽的关闭。 - 在赋值的目标位置上加一个负向前瞻,由
ASSIGN_SHAPE与CASE_ARM_SHAPE共用,拒绝GITHUB_OUTPUT=。限定在目标位置,因此被接受的 publish(>> "${GITHUB_OUTPUT}",一次读取)不受影响 —— 这是刻意不采用发现实测过会拒绝四个真实消费者的那种「扩大全 job 兄弟扫描」做法。GITHUB_ENV=/GITHUB_PATH=目标无需列入,因为全 job 扫描已经拒绝任何提到它们的步骤体。该前瞻对回溯是安全的:\s*只能吃空白,所以名字匹配无法滑过第一个字符来绕开它(M5/M6 双向变异验证)。
约束已守住。 现存 producer 的背书保住了:ci.yml 仍然是 problems=0 lanes=["ecs-qwen","ecs-win"]。四条子句对它零成本这一点在仓库内核实过 —— classify_pr 恰好两个步骤、都是 run: 步骤、没有 uses:、没有 container:、没有 defaults:,两个步骤体都不含 eval/declare/${!。既有的 container.env 钉扎继续通过;trusted_author=false 与 MYPATH=x 继续被背书;全机群保持 0 个问题。
未实施、已记录留给 maintainer: 该发现的「持久关闭方式」—— 「根本不要建模 shell」,接受右值为 ${ENV_NAME} 引用、经 envValue 解析到某个 env: 作用域里方括号字面量的 producer,并把 ci.yml 的两个标签数组移进 env:。见下方「后续跟进」。
已记录的后续跟进(不静默丢弃)
按 AGENTS.md 的「不要让评审轮次把 PR 撑大」规则(超过约 5 轮后只落地 Critical 修复,其余延后并留下记录),本轮 Critical 中的两条建议未实施:
- 在
vouchProducer中彻底不再建模 shell;把 ci.yml 的标签数组移进env:。 这是评审自己推荐的持久关闭方式,针对的是一个连续十一轮都再生出新接缝的发现家族 —— 它也是最有可能终结循环、而不是延长循环的改动。之所以没有单方面实施,是因为它不是机械修复:它要把pick_runner里基于AUTHOR_ASSOCIATION、EVENT_NAME、ECS_DISABLED、SAME_REPO、DISPATCH_LINUX_RUNNER的 bashcase/if决策换成 GitHub 表达式逻辑,这是一个关于真实 CI 路由如何选 runner 的风险决定,而不是守卫加固。属于 maintainer 的判断。 本轮改为关闭全部四个已实测的接缝,并逐条钉扎。 - 从
.github/actionlint.yaml派生REGISTERED_LANES。 R13-1 正是这种漂移的实例 —— 注册表写在分支上,从未得知 main 新增的标签,漂移窗口恰好一次合并那么宽。本轮按上述理由拒绝(它要在第二份文件里断言 runner 事实);如果 maintainer 接受这个取舍,值得作为后续跟进。
两份评审正文的收敛与残余风险部分都建议 land-with-residual-risk —— 即出口是 maintainer 的风险接受决定,而不是再开一轮评审。该建议是提给 maintainer 的,本轮不代为决定。
失败的检查
- Test (ubuntu-latest Node 22.x): FAILURE —— 根因即 R13-1。两个 HELPER_TESTS 步骤都会执行该守卫,且都没有
continue-on-error:ci.yml:436(GitHub CI helper checks,本 PR 这种只改.github的情形会归到github_ci_onlyprofile)与ci.yml:597(Run .github/scripts helper tests,fullprofile)。已修复,并用完全相同的 CI 命令在本地验证:658 项 / 658 通过 / 0 失败,退出码 0(HEAD 上有 8 项失败)。 - Post Coverage Comment (ubuntu-latest 22.x): FAILURE —— 同一根因的连带失败,是从 workflow 结构诊断出来的,不是假定的:
post_coverage_comment(ci.yml:1261)声明needs: [classify_pr, test],其第一个实质步骤下载产物coverage-reports-22.x-ubuntu-latest,而该产物由testjob 在ci.yml:749-753上传 —— 远在失败的 helper 检查步骤(ci.yml:428-436)之后。因此产物从未生成,下载失败。它不执行守卫的任何代码。不存在独立缺陷;预计随Test一起恢复。
改动
一个文件,位于本 PR 自身足迹内:.github/scripts/ci/runner-lane-labels.test.mjs(+284 / −64,全部为测试行;远低于本窗口 400 行的测试预算)。
以追加方式提交为 e890052423 —— ci: register the hk4 lane and close four guard fail-open seams —— 位于 origin/ci/short-job-lane 之上,标题风格与本分支前两次 autofix 轮次的提交一致。未做 amend、rebase、reset 或任何历史改写。提交后 git status 干净;工作树中不含任何临时或探针产物(全部探针与变异脚本都写在 /tmp 下,变异探针在 .github/scripts/ci/ 内需要的那一份临时副本已删除,且删除动作经过校验)。
一点环境说明:本次检出没有配置 git 身份,因此无法创建提交。已设置一个仅作用于本仓库(非 --global)的身份 qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> —— 与本分支前两次 autofix 轮次提交(31f1fc3a01、90f66378c6)所用的作者身份相同。除此之外没有改动仓库的任何配置。
未改动任何 workflow、CI 或验证机制、.husky/、.qwen/、仓库 scripts/、包源码、lockfile、workspace manifest 字段,也未改动 settings 源 —— 因此确实不需要执行 npm run generate:settings-schema。
--conflict false,故未合并 origin/main,分支保持检出时的状态。
验证
以下每条命令都实际执行过;没有任何结论是读 diff 读出来的。
node --test --test-concurrency=1 .github/scripts/ci/runner-lane-labels.test.mjs—— 152 项,152 通过,0 失败(HEAD 上为 139/131/8)node --test --test-concurrency=1 $HELPER_TESTS(即ci.yml:436与ci.yml:597执行的完全相同命令,清单取自ci.yml:82)—— 658 项,658 通过,0 失败,退出码 0npm run build—— 通过(退出码 0)npm run typecheck—— 通过(退出码 0)npm run lint—— 通过(退出码 0)npx prettier --check .github/scripts/ci/runner-lane-labels.test.mjs—— 干净。HEAD 的副本同样是干净的;本轮新增内容中有两处需要格式化,已格式化。(scripts/lint.js --prettier执行的是prettier --write .而不是检查,因此单独验证了这一项 —— 而那次全仓库写入重排了 37 个无关文件,已全部还原;提交的树里只有这一个预期文件。)npm run bundle后的集成测试 —— 未运行:此处没有任何行为是通过打包后的 CLI 或集成测试框架行使的。改动仅限于一个.github/scripts下的node:test守卫。npx vitest run scripts/tests/release-workflow.test.js—— 45 项,45 通过。运行它是为了确认 R13-1 选择修注册表而不是release.yml的判断::1563的ecsRunsOn钉住了那段完全相同的 runs-on 字符串,:1579的循环对八个 job 名逐一断言,改路由会让该套件变红。它是绿的,即路由未被触碰、钉扎依然有效。- 针对
packages/*的定向 Vitest —— 不适用:packages/下没有任何改动,且被改的套件用的是node:test,不是 Vitest。
变异探针 —— 本轮新增的每条子句都有自己的见证
每条子句都在一份临时副本上被移除或取反、重跑套件,随后源码被还原为逐字节相同(经程序化校验;临时副本已删除)。父级 suite 的汇总行已从计数中排除,因为 node 的 TAP 报告器在任何子测试失败时都会把 describe 标红。
| # | 被移除 / 取反的子句 | 预期变红 | 实测 |
|---|---|---|---|
| M1 | ecs-qwen-hk4-host 注册表条目 |
8 条 release.yml 子测试 |
8 ✓ |
| M2 | JSON.parse 权威 → 旧分词器(保留 parseOk 名称) |
5 条 R13-2 钉扎 | 5 ✓ |
| M3 | 终止 ` | 分支输出每个&&` 操作数 → 只取最后一个 |
|
| M4 | 向去括号递归传递终止性 | 1 条钉扎 | 1 ✓ |
| M5 | ASSIGN_SHAPE 中的 CONTROL_TARGET 前瞻 |
1 条钉扎 | 1 ✓ |
| M6 | CASE_ARM_SHAPE 中的 CONTROL_TARGET 前瞻 |
1 条钉扎 | 1 ✓ |
| M7 | 拒绝 producer job 带 container: |
2 条 container 钉扎 | 2 ✓ |
| M8 | 拒绝不透明的 uses: 兄弟步骤 |
1 条钉扎 | 1 ✓ |
| M9 | 兄弟解引用原语(${! / eval / declare) |
1 条钉扎 | 1 ✓ |
九条全部得到见证:每次变异恰好让自己的钉扎变红、不牵连其他,未变异基线为 152/152。
本轮新增钉扎(13 条)
R13-2(5 条):嵌套的 fromJSON 字面量 · 括号不配平的 fromJSON 字面量 · 嵌套的 env. 操作数 · 独立的嵌套 assembly · 两种非 JSON 分隔符载荷(单引号与配对不匹配)。
R12-1(3 条):终止 && 链中的空字符串前缀 · 终止 && 链中不可审判的前缀 · 非终止括号组保持其条件操作数不被审判(专门见证 M4 —— 只有终止性被传递时它才是绿的)。
R6-1(5 条):不透明的 uses: 兄弟 · 通过解引用构造出口名字的兄弟 · 运行在 container 中的 producer job · 外加 residualEntrances 表中两行 GITHUB_OUTPUT 重绑定(赋值形式与 case 分支形式)。
以上每一条在 HEAD 上都产生 problems=0,因此移除对应修复后都会变红。
🦷 Gate advisory — this round resolves a Critical/Request-changes finding with test-only changes (machine-measured): the bite check cannot verify a test-side fix, so the resolution rests on the round summary alone. · 本轮以纯测试改动解决 Critical/Request-changes 反馈(门自动测量):bite 检查无法验证测试侧修复,该解决仅以轮次摘要为凭。
Deferred non-Critical feedback
Critical-only mode is active: the round counter reached 5 (this window was seeded at round 10 by @qwen-code /takeover from 10, plus 2 change-producing round(s) since). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (@qwen-code /retry starts a fresh counting window.)
中文说明
已进入仅处理 Critical 的模式:轮次计数已达 5(本窗口由 @qwen-code /takeover from 10 从第 10 轮起算,此后又完成 2 个产生改动的轮次)。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 @qwen-code /retry 可开启新的计数窗口。)
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
🧵 Resolved all 6 selected review thread(s). · 已关闭全部选中的 6 条评审线程。
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-2026-09-02
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
13 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- MAINTENANCE_LABEL's ecs-update- prefix exemption accepts any invented suffix (.github/scripts/ci/runner-lane-labels.test.mjs:86) — already reported as R4-11 (round-4 deferral, review 5067140559)
- PLATFORM_LABELS omits the auto-assigned arm/arm64/macos labels (.github/scripts/ci/runner-lane-labels.test.mjs:80) — already reported as RA1-2 (round-3 deferral, review 5064059149), re-confirmed as RA-7 in round 7
- no lane-to-platform binding, so a Windows platform set carrying a Linux-only lane is vouched (.github/scripts/ci/runner-lane-labels.test.mjs:395) — already recorded in the round-13 deferral list (review 5088415184), and as RA-10 (round-7 de…
- matrix./env. reference resolution implemented three times (.github/scripts/ci/runner-lane-labels.test.mjs:587) — already reported as D5-1 (round-5 deferral, review 5069369299), re-confirmed as R7-8 and R12-A
- the unquoted-residue branch is written three times and its messages have drifted (.github/scripts/ci/runner-lane-labels.test.mjs:609) — already recorded in the round-13 deferral list (review 5088415184)
- pick_runner tripwire message misdiagnoses a new producer variable (.github/scripts/ci/runner-lane-labels.test.mjs:1067) — already reported as D11-3 (round-11 deferral, review 5083676054)
- registeredEnforcementTests counts registrations, not verdicts (.github/scripts/ci/runner-lane-labels.test.mjs:2095) — already reported as R4-10 (round-4 deferral, review 5067140559), re-confirmed as DUP-1 / V-3 / R7-6 / R12-C
- the guard's HELPER_TESTS membership is pinned by nothing (.github/workflows/ci.yml:82) — already reported as D6-2 (round-6 deferral, review 5071661168), re-confirmed as R12-B
- every uses: job is skipped unconditionally, so a cross-repo callee's runs-on is vouched unread (.github/scripts/ci/runner-lane-labels.test.mjs:996) — already reported as D11-5 (round-11 deferral, review 5083676054)
- a second hand-kept label registry diverging from .github/actionlint.yaml (.github/scripts/ci/runner-lane-labels.test.mjs:66) — already put to the author as R13-1's durable form (comment 3913098451)
- valueArms' paren-strip mangles doubly-wrapped expressions (.github/scripts/ci/runner-lane-labels.test.mjs:211) — already reported as R7-3 (round-7 deferral, review 5073874414)
- the standalone-assembly self-hosted prefilter is unpinned (.github/scripts/ci/runner-lane-labels.test.mjs:1008) — already reported as R7-7 (round-7 deferral, review 5073874414)
- the residual allowlist has no shape for inert honest shell lines such as set -euo pipefail (.github/scripts/ci/runner-lane-labels.test.mjs:291) — already recorded in the round-13 deferral list at :254 (review 5088415184)
Not reviewed: reverse audit — stopped before round 8 by the review time budget.
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:658 — [review] matrix/env resolution recurses with no depth or cycle guard, so a cyclic reference aborts the registration loop.github/scripts/ci/runner-lane-labels.test.mjs:544 — [review] eleven reachable fail-closed branches have no test; a mutation sweep leaves the suite at 152/152 for each.github/scripts/ci/runner-lane-labels.test.mjs:713 — [review] standalone-assembly diagnostic drops the variable name, job and step.github/scripts/ci/runner-lane-labels.test.mjs:1063 — [review] tripwire hardcodes ci.yml's self-hosted assignment count in a messageless assert.github/scripts/ci/runner-lane-labels.test.mjs:906 — [review] the new (?:eval|declare) sibling-scan arm is load-bearing but unpinned, and invisible to line coverage.github/scripts/ci/runner-lane-labels.test.mjs:459 — [review] scalar and non-self-hosted branches never consult the registry, so a registered lane is refused with a false diagnostic.github/scripts/ci/runner-lane-labels.test.mjs:537 — [review] the fromJSON recursion drops the new terminality parameter, judging a condition prefix GitHub swallows.github/scripts/ci/runner-lane-labels.test.mjs:336 — [review] matrixValues never applies matrix.exclude, so removed legs are judged and booked as lane references
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 13 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:反向审计——评审时间预算不足,未能开始第 8 轮。
收敛姿态下延后(第 14 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
…hape
The required Test check was red on the merged tree: main's trusted-base
classifier change added an actions/checkout to classify_pr AFTER
pick_runner publishes, and the guard's sibling scan refused the whole job
for it, reddening all four consumers of ubuntu_runner. A job runs its
steps in order, so only earlier ones can plant a step's environment;
scoping the scan to them keeps the closure and vouches the live chain.
R6-1's five entrances are closed by observing the value instead of
inferring it from the file's text. A shared harness runs a producer body
under bash with a clean environment and reads $GITHUB_OUTPUT — the hop the
consumers actually read — and the guard executes the body across the whole
routing input matrix, seeded with a poisoned value for the consumed name
so a body that publishes what it inherited is refused. A `${{ }}` in the
body is refused outright, because GitHub substitutes it before bash parses
and no reader of the file can say what runs; the assembly scan drops such
lines for the same reason. Two steps sharing the producer id now fail
closed instead of resolving through `.find()`.
The static residual allowlist stays. Replacing it with execution alone was
measured first and turns 22 existing fail-closed pins green: a body that
delegates to `source ./evil.sh` runs clean when the file is absent. The
two vouches are complementary — each refuses what the other cannot see.
ci-runner-routing.test.mjs imports the same harness and now asserts the
published output rather than the stdout line the step echoes beside it.
Eight new pins, one per entrance and one per added branch, each red at the
previous commit and red again under its own reverting mutation.
|
🤖 Addressed the latest review feedback (round 14/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 14/100 轮)。改动内容与我反驳保留之处如下: Autofix round report — PR #10575 (round 15, Critical-only)Two actionable items reached this round: one Critical inline finding
The failing required check — reproduced, not guessedThe check could not be read from logs (this round has no GitHub credentials), Cause: Fix: a job runs its steps in order, so only steps before the producer can R6-1 — the five entrancesThe finding's core claim is confirmed: because GitHub substitutes
The execution harness the finding pointed at is now real and shared: Where this round diverges from the finding's preferred remedy — and whyThe finding preferred deleting the static model outright ( For the same reason the finding's "refuse a sibling The finding's static-path bars that are independent of that choice were all Diff growth — disclosed3 files, +442/−50. The new harness module is ~116 source lines; the two test VerificationEvery command below was actually run in this checkout.
中文说明Autofix 轮次报告 —— PR #10575(第 15 轮,仅处理 Critical)本轮有两项可执行内容:一条 Critical 行内发现(
失败的必需检查 —— 已复现,而非猜测本轮没有 GitHub 凭据,无法读取 CI 日志,因此在本地复现。守卫套件在 PR 分支单独运行时是通过的(152/152),这正是上一轮看起来绿灯的原因。它在 CI 真正构建的那棵树上失败 —— 即 PR head 与当前 原因: 修复:一个 job 的步骤是按顺序执行的,因此只有 producer 之前的步骤才可能植入它的环境或命令查找。扫描被限定为 R6-1 —— 五个入口该发现的核心论断成立:由于 GitHub 会在 bash 解析之前把
该发现指出的执行 harness 现已真实存在且被共享: 本轮与该发现首选方案的差异 —— 以及原因该发现更倾向直接删掉静态模型( 出于同样原因,该发现提出的"拒绝委派给未读代码的兄弟 该发现静态路径中与上述选择无关的屏障全部已实现:body 级 Diff 增长 —— 主动披露3 个文件,+442/−50。新 harness 模块约 116 行源码;两个测试文件净增约 276 行测试代码,使本计数窗口超出其 400 行测试预算(此前为 220)。增长来自该发现验收标准所要求的八条有见证的钉扎,加上执行这一跳;本轮没有任何推测性内容。未触及 PR 自身 验证以下每条命令都在本 checkout 中实际运行过。
Deferred non-Critical feedbackCritical-only mode is active: the round counter reached 5 (this window was seeded at round 10 by 中文说明已进入仅处理 Critical 的模式:轮次计数已达 5(本窗口由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 1 selected review thread(s). · 已关闭全部选中的 1 条评审线程。 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/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- valueArms paren-strip mangles doubly-wrapped expressions — already reported as R7-3 (round-7 deferral, review 5073874414)
- REGISTERED_LANES vs .github/actionlint.yaml drift (ecs-light not added there) — already put to the author as R13-1's durable form (comment 3913098451)
- matrix./env. reference resolution implemented three times — already reported as D5-1 (round-5 deferral, review 5069369299)
- registeredEnforcementTests counts registrations, not verdicts — already reported as R4-10 (round-4 deferral, review 5067140559)
Not reviewed: reverse audit — stopped before round 10 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 15, not a blocker) — recorded, not requested in this round:
.github/scripts/ci/runner-lane-labels.test.mjs:596 — [probe] vouchProducer re-runs the 864-spawn hop 4 for every consumer of the same output — memoize per (producerJob, outName) (~3.9 s/run).github/scripts/ci/runner-lane-labels.test.mjs:365 — [probe] envValue's job-over-workflow scope precedence is unpinned — the inverted-scope mutant survives 160/160.github/scripts/ci/runner-selection-harness.mjs:46 — [probe] DISPATCH_RUNNERS never drives the declared 'hosted' dispatch option of ci.yml's linux_runner input.github/scripts/ci/runner-lane-labels.test.mjs:941 — [probe] rationale comments cite a classify_pr actions/checkout step that does not exist at this commit.github/scripts/ci/runner-selection-harness.mjs:37 — [probe] EVENT_NAMES omits 'schedule' (ci.yml fires it nightly) while driving 'issues', which no producer fires.github/scripts/ci/runner-lane-labels.test.mjs:891 — [probe] env-scope scan checks membership only — a binding remapping a driven key's value escapes the executed matrix
Convergence: round 15 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (0 new). Findings keep coming back to the same files: .github/scripts/ci/runner-lane-labels.test.mjs (findings in round 6; 1 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.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (1 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 0), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:反向审计——评审时间预算不足,未能开始第 10 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-force-push-reminder.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/pr-self-report-label.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage-finalize.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 15 轮,非阻断)——已记录,本轮不要求修改:共 6 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 15 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 0 条首次提出)。发现反复回到同一批文件:.github/scripts/ci/runner-lane-labels.test.mjs(第 6 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 1 条 Critical),首次发现的速率没有下降(本轮 1,上一轮 0),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| // the publish expand a value no scannable literal assigns, and a | ||
| // BASH_ENV/ENV/BASH_FUNC_* binding is executed by bash at startup, | ||
| // before the body the guard read (R6-1 h/l). | ||
| const envScopes = [step.env, job.env, doc?.env]; |
There was a problem hiding this comment.
[Critical] R6-1: (fix-induced) [certifies-falsely] [new-surface] still stands — vouchProducer's closed chain still fails open as a class. The five entrances round 14 recorded are genuinely closed at this commit — this round re-probed each against HEAD and all fail closed (the body ${{ }} bar, the assembly-comment scanner filter, the bash-delivered double-quoted judgment, the duplicate-id filter, and the poisoned-seed refusal). What the round-15 fix added to close them — hop 4's executed vouch and its shared harness — opens new entrances of the same root, because the execution observes the body under conditions that differ from GitHub's evaluation. Demonstrated this round by execution: (1) a producer job binding env: EXTRA: 'x' with its assignment guarded by if [[ "${EXTRA}" != "x" ]] is vouched with problems=[] and the lane booked, while in CI the bound value makes the branch dead, nothing publishes, and every consumer silently takes its hosted fallback — and the mirror direction (== "x", branch live in CI) gets the poison-seed refusal, a false red on a healthy producer; the env-scope scan checks only membership, and hop 4 drives only the five driven keys; (2) a step-level if: (e.g. if: github.event_name != 'push' on the producer) is never read by any hop — on the skipped event the output serves '' and consumers fall back hosted while the guard stays green; (3) [[ ]] conditions testing state the harness does not reproduce — -o errexit (off under the harness's plain bash -c, on under GitHub's bash --noprofile --norc -e -o pipefail {0}) and -f file tests (harness cwd vs the runner's workspace) — are vouched whichever side the harness happens to take. pick_runner binds exactly the five driven keys and carries no if: today, so nothing misroutes today; this is the guard's forward contract failing open on the fix round that claimed to close the class — closing entrances one at a time cannot close it, because the divergence is between two systems (the harness's execution context and GitHub's), not in any enumerable list of spellings. The exported DRIVEN_ENV_KEYS constant already names the boundary of entrance (1) and has zero read sites. Close the class where the divergence lives: fail closed when the body reads an env key that is bound in a visible scope but not driven by routingEnvironments() (a read-site-scoped bar, not a flat per-scope bar); refuse a producer step carrying an if: key; and bar [[ ]] conditions that test state the harness cannot reproduce (option tests, file tests, $$/$RANDOM-class expansions) — or execute hop 4 under GitHub's actual invocation and workspace assumptions. Wire the first bar through DRIVEN_ENV_KEYS so the list and the check cannot drift.
Witness:
env-fidelity probe (HEAD): problems=[] lanes=["ecs-qwen"] <- false vouch
refined fix (refuse reads of bound-but-undriven keys): flips to refusal; live ci.yml stays green
mirror (== "x"): poison-seed refusal on the healthy producer (false red)
if:-skip probe: BASE ok 49 ...names a registered lane (# pass 161 / # fail 0)
-> FIX not ok 49 ...unreadable write (# fail 1)
condition-state probe: ERREXIT-PROBE [] ["ecs-qwen"]; FILETEST-PROBE [] ["ecs-qwen"]
bash -c -> BRANCH-TAKEN; bash --noprofile --norc -e -o pipefail -c -> BRANCH-SKIPPED
round-14 entrances re-probed at HEAD: all five fail closed
The fix must not violate the live chain: pick_runner binds exactly the five driven keys at step scope (ci.yml:169-175) and carries no if: — a flat per-scope refusal reddens the repo's own main CI lane over workflow-level bindings the body never reads (measured: ACTIONLINT_VERSION & co), and a flat job-level-if: refusal reddens classify_pr (job-level if: at ci.yml:101); scope both bars accordingly. Acceptance criterion: one pin per entrance in the fail-closed describe — the env-bound dead-branch producer, the producer carrying if: github.event_name != 'push', and the sole assignment behind [[ ! -o errexit ]] — each red at HEAD, green under the fix, and red again when its own clause is removed (every flip was measured at this commit).
中文说明
[Critical] R6-1:(修复引入)[certifies-falsely] [new-surface] 依然存在——vouchProducer 的闭环链作为整体仍然敞开失败。第 14 轮记录的五个入口在本提交上确实都已关闭——本轮对 HEAD 逐一重跑探针,五者全部失败即报(body 级 ${{ }} 屏障、assembly 行尾注释的扫描级过滤、按 bash 实际交付值审判的双引号赋值、重复步骤 id 的 filter 检查、投毒种子拒绝)。但第 15 轮为关闭它们而新增的东西——hop 4 的执行背书及其共享 harness——打开了同一根因的新入口:执行观察 body 时所处的条件与 GitHub 的真实求值条件并不一致。本轮已实际执行演示:(1) 一个 producer job 绑定 env: EXTRA: 'x'、且赋值由 if [[ "${EXTRA}" != "x" ]] 守护时,守卫以 problems=[] 背书并登记该 lane;而在 CI 中,绑定值使该分支成为死分支,什么都不发布,每个消费者静默退回托管 fallback——镜像方向(== "x",分支在 CI 中有效)却得到投毒种子拒绝,对一个健康 producer 误报红;env 作用域扫描只检查成员资格,hop 4 也只驱动那五个被驱动的键;(2) 步骤级 if:(例如在 producer 上加 if: github.event_name != 'push')任何一跳都不读——在被跳过的事件上输出为 '',消费者退回托管,守卫却保持绿色;(3) [[ ]] 条件测试了 harness 无法复现的状态——-o errexit(harness 的裸 bash -c 下关闭,GitHub 的 bash --noprofile --norc -e -o pipefail {0} 下开启)与 -f 文件测试(harness 的 cwd 与 runner 的工作区)——守卫按 harness 恰好取到的那一侧背书。今天的 pick_runner 恰好只绑定那五个被驱动的键、也没有 if:,所以今天没有任何误路由;问题在于守卫的前向契约在声称已关闭该类的修复轮上再次敞开失败——逐个封堵入口无法关闭这个类,因为分歧存在于两个系统之间(harness 的执行上下文与 GitHub 的执行上下文),而不是任何可枚举的写法清单里。已导出的 DRIVEN_ENV_KEYS 常量本就命名了入口 (1) 的边界,却没有任何读取点。请在分歧所在处关闭该类:当 body 读取了一个在可见作用域中被绑定、但未被 routingEnvironments() 驱动的 env 键时失败即报(按读取点限定的屏障,而不是按作用域一刀切);拒绝携带 if: 键的 producer 步骤;并禁止测试 harness 无法复现状态的 [[ ]] 条件(选项测试、文件测试、$$/$RANDOM 一类展开)——或者让 hop 4 在 GitHub 的真实调用方式与工作区假设下执行。请把第一道屏障接在 DRIVEN_ENV_KEYS 上,使清单与检查不会漂移。
修复不得破坏现存链路:pick_runner 在步骤作用域恰好绑定五个被驱动的键(ci.yml:169-175)且没有 if:——按作用域一刀切的拒绝会因 body 从未读取的工作流级绑定(实测:ACTIONLINT_VERSION 等)把仓库自己的主 CI lane 染红;对 job 级 if: 一刀切拒绝会染红 classify_pr(job 级 if: 在 ci.yml:101);两道屏障都必须按此限定范围。验收标准:在失败即报 describe 中为每个入口各加一条钉扎——绑定了 env 的死分支 producer、携带 if: github.event_name != 'push' 的 producer、以及唯一赋值位于 [[ ! -o errexit ]] 之后的 producer——每条在本 HEAD 为红、加修复后为绿、移除自身子句后再次变红(每次翻转均已在本提交上实测)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🔀 Base updated: red check(s) [Dependency CVE audit, Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Dependency CVE audit, Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |






What this PR does
Eight jobs that finish in seconds move from the
ecs-qwenlane to a newecs-lightlane on the same self-hosted pool:Remind on force-pushpr-force-push-reminder.ymlfinalize-triage-ciqwen-triage-finalize.ymllabelpr-self-report-label.ymlauthorizeqwen-code-pr-review.ymlack-review-requestqwen-code-pr-review.ymlreview-configqwen-code-pr-review.ymlauthorizeqwen-triage.ymlPublish serve A/B to the PRserve-ab-publish.ymlThe fleet dedicates each of its five Linux hosts to a lane, so the pool stays assignable at a glance:
The label was added and verified on the runners before this change, so no job can land on a lane that does not exist.
Also adds
.github/scripts/ci/runner-lane-labels.test.mjs, a guard that fails when a workflow asks for a self-hosted lane the registry does not list, and when a registered lane loses its last consumer.Why it's needed
These jobs shared a lane with the 90-minute Test job, so they queued behind it. A snapshot taken during the 2026-08-30 saturation: 121 jobs waiting on
ecs-qwen, ~74% of them these seconds-long jobs, median wait 27 minutes, p90 49 minutes.Measured across a day of runs, they are 71% of the job count but 13% of the machine time. The delay is head-of-line blocking, not capacity — which is why a separate lane fixes it and adding runners did not: a batch of 10 extra
ecs-qwenslots was absorbed instantly and left the median wait unchanged.Two of these are gates, so the blocking compounds:
authorizedecides whether a review runs at all, and it spent 27 minutes queued to do 26 seconds of work.Why not route them to
ubuntu-latestThat was the first thing I tried, and it is wrong. The comment above each of these
runs-onlines says the job "checks out nothing and runs no repository code … so the persistent ECS pool is safe and skips the saturated hosted queue" — moving to the hosted pool was the thing these jobs were moved away from.ci-runner-routing.test.mjs:615records the incident behind that: on 2026-08-25 a hosted backlog queued autofix'sroute/review-scanpast their cron period, and the supersede rule then starved every scan round.ecs-lightkeeps the original rationale intact and only removes the queueing.Scope
Deliberately not included:
classify_pr(ci.yml) — it publishesubuntu_runnerfor every downstream Linux job, andci-runner-routing.test.mjsasserts itsruns-onexpression agrees with its ownpick_runnerstep. Splitting it needs its own design.route/review-scan— covered by the 2026-08-25 regression guard above; changing their lane means re-arguing that test.Noted for follow-up, not addressed here:
qwen-triage'striageandverifyjobs (19 min median, 151 min max) run onecs-qwenand compete with CI for the 90-minute lane. They look likeecs-agentwork.Reviewer Test Plan
How to verify
node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs .github/scripts/ci/runner-lane-labels.test.mjs— 61 pass, 0 fail.REGISTERED_LANES→"<label>" is not a registered lane;ecs-lightreference →"ecs-light" is registered here but no workflow asks for it.bash .github/scripts/check-workflow-size.sh— passes; no file moves more than 4096 bytes from its baseline.authorizeandlabelstarting within seconds, onecs-qwen-runner-64c-1,-sg-1or-hk-j6cdqefjt78v0s9jngns-1.Verification notes
The full
HELPER_TESTSsuite reports 7 failing files both on this branch and on a cleanorigin/maincheckout (they neednode_modulesthat a bare worktree lacks);qwen-triage-workflow.test.mjsreports the same 36 failures on both. No new failures.prettier --checkandeslint --max-warnings 0are clean on the changed files.Tested on
Risk & Scope
timeout-minutes: 5(includingreview-config, which had no cap until review pointed it out) and two at 10 (finalize-triage-ci,serve-ab-publish). Oneauthorizesample in the measurement window ran 3085 s, which its own 5-minute cap says should be impossible — worth a look, but it is bounded either way.ecs-qwenandecs-agentare otherwise untouched by this PR.MAINTAINER_ECS_RUNNER_DISABLEDstill routes every one of these toubuntu-latestunchanged.中文说明
本 PR 的改动
八个几秒钟就结束的 job 从
ecs-qwen迁到同一个自建池上的新 laneecs-light:Remind on force-pushpr-force-push-reminder.ymlfinalize-triage-ciqwen-triage-finalize.ymllabelpr-self-report-label.ymlauthorizeqwen-code-pr-review.ymlack-review-requestqwen-code-pr-review.ymlreview-configqwen-code-pr-review.ymlauthorizeqwen-triage.ymlPublish serve A/B to the PRserve-ab-publish.yml机群把五台 Linux 宿主机按 lane 整台专用,池子一眼可读:
标签是在本 PR 之前就打好并验证过的,所以不会出现 job 落到不存在的 lane 上。
另外新增
.github/scripts/ci/runner-lane-labels.test.mjs:当 workflow 请求一个注册表里没有的 self-hosted lane、或某个已注册 lane 失去最后一个消费者时,它会失败。为什么需要
这些 job 和 90 分钟的 Test job 共用一条 lane,于是排在它后面。2026-08-30 饱和期间的快照:121 个 job 在等
ecs-qwen,其中约 74% 是这些秒级 job,等待中位数 27 分钟,p90 49 分钟。按一整天的运行数据统计,它们占 71% 的 job 数,但只占 13% 的机时。所以延迟来自队头阻塞而非容量不足——这也解释了为什么单独分 lane 有效、而加机器无效:之前给
ecs-qwen补了 10 个槽位,瞬间被吃满,等待中位数纹丝不动。其中两个还是门控 job,阻塞会逐级放大:
authorize决定 review 要不要跑,它排了 27 分钟队,只为做 26 秒的活。为什么不直接改成
ubuntu-latest这是我最初的方案,是错的。这些
runs-on上方的注释写着:该 job「不检出任何东西、不运行仓库代码……所以持久池是安全的,而且能绕开饱和的托管队列」——托管池正是它们当初被搬离的地方。ci-runner-routing.test.mjs:615记录了背后的事故:2026-08-25 托管积压把 autofix 的route/review-scan拖过了 cron 周期,supersede 规则接着饿死了每一轮 scan。ecs-light保留了原有理由,只消除排队。范围
刻意不含:
classify_pr(ci.yml)——它为所有下游 Linux job 发布ubuntu_runner,而ci-runner-routing.test.mjs断言它的runs-on表达式必须与自身的pick_runner步骤一致。拆分它需要单独设计。route/review-scan——受上述 2026-08-25 回归守卫保护,改动 lane 意味着要重新论证那条测试。记录为后续、本 PR 不处理:
qwen-triage的triage和verify(中位 19 分钟,最长 151 分钟)跑在ecs-qwen上,与 CI 争抢 90 分钟的 lane。它们看起来更像ecs-agent的活。审查测试计划
如何验证
node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs .github/scripts/ci/runner-lane-labels.test.mjs—— 61 通过,0 失败。REGISTERED_LANES之外的标签 →"<label>" is not a registered lane;ecs-light引用 →"ecs-light" is registered here but no workflow asks for it。bash .github/scripts/check-workflow-size.sh—— 通过,没有文件偏离基线超过 4096 字节。authorize和label应在数秒内开跑,落在ecs-qwen-runner-64c-1、-sg-1或-hk-j6cdqefjt78v0s9jngns-1上。验证说明
完整的
HELPER_TESTS套件在本分支和干净的origin/main检出上都报同样 7 个文件失败(它们需要裸 worktree 没有的node_modules);qwen-triage-workflow.test.mjs在两侧同样报 36 个失败。无新增失败。改动文件的prettier --check与eslint --max-warnings 0均干净。测试平台
风险与范围
timeout-minutes: 5(含review-config——评审指出它此前没有上限,本次补上),两个 10 分钟(finalize-triage-ci、serve-ab-publish)。测量窗口里有一个authorize样本跑了 3085 秒,按它自己 5 分钟的上限这不该发生——值得看一眼,但无论如何是有界的。MAINTAINER_ECS_RUNNER_DISABLED仍会把这八个 job 原样打回ubuntu-latest。