feat(triage): add a deterministic flakiness gate to sandboxed verification - #9130
Conversation
…ation Closes #9125. PR #9086's ~50% mtime-assertion flake passed every automated layer because each executed the changed tests exactly once — a coin flip a single green run cannot distinguish from health. The gate re-runs the PR's added/modified unit-test files N times (default 5, vars.QWEN_VERIFY_FLAKE_ROUNDS to override, clamped to 2..10) through the same entry points CI uses and compares outcomes per group across rounds. Design constraints, each pinned by a workflow test: - One-way authority: 'flaky' demotes the published headline (even a trusted agent merge-ready); no gate value can raise or soften one. The gate runs the PR's own test code, so it can always be neutered — but a gate that can only demote is not worth forging. - Divergence-only signal: a group failing identically every round is deterministic (CI owns it) and an environment-sensitive suite must not false-positive here; both report informationally, never demote. - Fail open: the gate is not under -e and every terminal path exits 0 — a gate bug reports verdict 'error' instead of taking down the verify lane. - Honest file list: recorded from HEAD^1..HEAD before install/build hands the workspace (and .git) to PR lifecycle code; the gate consumes the root-owned recorded list and never re-derives the diff. - Untrusted text stays out of outputs: summaries are fixed text plus counters; PR-controlled paths live in flake-gate.log, embedded through the publisher's escaping emit_block. Job timeout raised 150 -> 175 for the gate's ~25m worst case (15m round budget checked before each invocation + one 10m-capped in-flight run).
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@wenshao The PR body is missing required headings from the PR template: Reviewer Test Plan (How to verify / Evidence / Tested on), Risk & Scope, and Linked Issues — only What this PR does and Why it's needed are present.
Most of the content is already there under non-template headings — Verification reads as the reviewer test plan, and Closes #9125 is stated inline — so this is a restructure rather than new work. The one section that is substantively worth writing for a verify-lane change is Risk & Scope: the main tradeoff (the timeout bump 150 → 175 min and up to ~25 minutes added to a verify run in the worst case, both per the PR body), what is not validated / out of scope, and migration notes (e.g. vars.QWEN_VERIFY_FLAKE_ROUNDS only needs to be set to override the default of 5 rounds).
Please align the body with the template, then re-trigger with @qwen-code /triage.
中文说明
@wenshao PR 正文缺少 PR 模板要求的小节:Reviewer Test Plan(How to verify / Evidence / Tested on)、Risk & Scope、Linked Issues——目前只有 What this PR does 和 Why it's needed。
内容其实基本都在,只是挂在非模板标题下——Verification 实际上就是 reviewer 测试计划,Closes #9125 也写在了正文里——所以只需重排结构,不用补写新内容。对这类 verify 通道改动,真正值得补的是 Risk & Scope:主要权衡(超时 150 → 175 分钟、最坏情况下单次 verify 增加约 25 分钟,均引自 PR 正文)、未验证/超出范围的部分,以及迁移说明(例如 vars.QWEN_VERIFY_FLAKE_ROUNDS 仅在需要覆盖默认 5 轮时才要配置)。
请按模板调整正文后,用 @qwen-code /triage 重新触发。
— Qwen Code · qwen3.8-max
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 /triage |
|
Sandboxed verification: ❌ not passed — blocked (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: 186 passed · 2 failed · 188 total 中文 — 判定:❌ 不通过 · 阻塞(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:186 通过 · 2 失败 · 188 总计 Verification reportPR #9130 Deep Verification — feat(triage): add a deterministic flakiness gate to sandboxed verificationVerdict: The 2 failures are the central claim's A/B cells, not harness accidents: the gate script works exactly as designed under the shell contract the author's harness used, and breaks under the shell contract this lane's runner actually applies (observed live, Section 3). 中文 — 判定:blocked · 抖动门在生产 shell 约定下于首个失败用例处即被 -e 杀死
1. ScopeCentral claim: the flakiness gate re-runs the PR's changed test files N rounds, and run-to-run divergence yields Secondary claims: (1) honest file list — recorded from 2. Secondary claims: verified
The PR's own suites at head: 3. Central claim: A/B — the production shell contract defeats the gateThe decisive runtime fact (measured, not inferred)This container is the lane's own runtime. The live process tree of this verify run contains the runner's step wrapper: Every ( cd "${group_dirs[$i]}" && timeout -k 30 600 runuser -u node -- … bash -c "${group_cmds[$i]}" ) > "$out" 2>&1
status=$?is a bare compound command: under errexit, the first failing test invocation kills the script before Cell table (harness:
|
| # | Scenario | Author contract (bash --noprofile --norc) |
Production contract (… -e -o pipefail) |
|---|---|---|---|
| A | all groups pass, 5 rounds | exit 0, pass, matrix 4×PPPPP ✅ |
identical ✅ (control) |
| B | one group FFFFF |
exit 0, consistent-fail ✅ |
— (consumed by C) |
| C | same FFFFF scenario |
exit 0, consistent-fail |
exit 1, no outputs, log dies mid-round ❌ |
| D | one group PFPFP |
exit 0, flaky, matrix PFPFP ✅ |
exit 1, no outputs (dies at the first F, round 2) ❌ |
| E | empty list / missing list | — | exit 0, n/a / error ✅ |
| F | FLAKE_ROUNDS ∈ {1,0,11,abc,-3,''} → {2,2,10,5,5,5} rounds |
all ✅ (clamp correct) | (wrapper-orthogonal) |
| G | timeout branches (deadline constant sed-shrunk — noted variant) | both branches ✅ informational | — |
Witness: 02-gate-ab-author-vs-prod-wrapper.png (full 47-check run, 45/2).
Consequence chain in production (each link verified against the extracted YAML): the first failing invocation → flake step exits non-zero → no continue-on-error → job red → Install evidence browser and Run verification agent skipped (their ifs carry no always(); the workflow's own idiom — always() && on every step meant to survive failure, e.g. Upload verify results — corroborates that a bare custom if does not) → verdict/agent_verdict/flake_verdict all empty → publisher's *) arm prints "flaky and consistent-fail populations the gate exists to classify. For them the PR converts today's working lane into a red step + empty inconclusive comment. pass/n/a/timeout (without failures)/error still work — the harmless half.
Ironic note: the PR's own mutation list names "gate under set -euo" as one of the four seeded defects its tests catch — yet that is exactly what the runner wrapper applies, invisibly to YAML-parsing tests (matrix row M9 below). The author's stub harness plainly ran bash gate.sh without the wrapper — an uncalibrated replay.
4. Mutation matrix (PR's own tests) — mutation-harness.mjs, witness 04-mutation-matrix.png
Positive control: unmutated scratch tree 61/61 green. Each mutant applied to a scratch copy with unique anchors; failures quoted from TAP.
| Mutant | Expected | Measured |
|---|---|---|
M1 gate under set -euo (seeded) |
kills 'runs PR test code… fails open' | ✅ exactly 1; message: 'the gate must not run under -e' |
| M2 log copy-back removed (seeded) | kills 'exposes the gate outcome… preserves its log' | ✅ exactly 1 |
M3 QUAL='✅ passed' in pass) arm (seeded) |
kills 'gate authority is one-way' | ✅ exactly 1 |
| M4 record step moved after install/build (seeded) | kills 'records the changed-test list BEFORE' | ✅ exactly 1 |
M5 git diff string added to gate run |
kills BEFORE test (doesNotMatch) |
✅ exactly 1 |
| M6 timeout-minutes 175→150 | kills timeout-budget test | ✅ exactly 1 |
M7 GITHUB_TOKEN: 'leaked' |
kills no-tokens test | ✅ exactly 1 |
M8 proposed fix (set +e) |
suite stays green | ✅ 61/61 |
M9 production defect present (wrapper -e) |
— | suite also 61/61 green — structural blindness: no test in the PR can observe the wrapper |
The four seeded defects each fail exactly one of the five new tests, as the description claims — the tests have teeth within their observable surface. The wrapper defect sits outside that surface; gate-harness.mjs (cells C/D) is the fixture that goes red without the fix and green with it.
5. Findings
5.1 Blocker — the gate runs under the wrapper's -e; fail-open inverts to fail-closed at the first failing test
See Section 3 for the full A/B. Reproduce:
node tmp/pr9130-verify-20260814-053200/gate-harness.mjs # sections C and D go NOT OKor minimally: bash --noprofile --norc -e -o pipefail extracted/flake-step.sh with a flake-gate-files listing one test whose runner exits 1 — the step dies with that exit code, $GITHUB_OUTPUT stays empty.
Blast radius: any PR touching unit tests where any invocation fails in any round (flaky or deterministically failing) → red gate step → agent skipped → inconclusive comment without report. Deterministically-failing changed tests today get a normal verify run; with this PR they get a dead lane. The demotion feature never fires because divergence requires an F, and the first F kills the step.
Suggested fix (measured, one line): add set +e after the set -uo pipefail line — it realizes the step's stated intent ("Deliberately NOT -e") regardless of wrapper. Measured in harness section H under the production wrapper: consistent-fail, flaky (PFPFP), late-divergence (PPPPF), and total runner collapse (runuser exit 127 every call) all land as their intended informational verdicts with exit 0; the benign all-pass fixture is byte-identical (outputs and log) to the unfixed author-contract run; the suite stays 61/61 (M8). The fix should ship with its fixture: cells C/D of gate-harness.mjs (or a workflow-test variant that runs the extracted script under bash -e -o pipefail) — nothing in the current suite pins this axis.
5.2 Note — web-shell E2E specs are recorded but never runnable → permanent consistent-fail noise
The record step's (test|spec) pattern captures the 12 packages/web-shell/client/e2e/*.spec.ts files; the partition maps them to the packages/web-shell vitest group; that config excludes e2e/**. Measured: cd packages/web-shell && npx --no-install vitest run client/e2e/web-shell.smoke.spec.ts → No test files found, exiting with code 1. So a PR touching those specs gets FFFFF… → consistent-fail every time — informational only, never demoting, but the gate "re-ran" zero tests while reporting a deterministic failure. Consider logging them out-of-gate-scope like integration-tests/*.
5.3 Note — the embedded gate log is display-integrity-weak (two ways, both bounded)
(a) FLAKE_LOG="$(find verify-results -type f -name 'flake-gate.log' | sort | head -1)": the agent step also copies the workspace's tmp/*-verify-* dirs into verify-results, so a PR-controlled nested flake-gate.log under a dirname sorting before f (e.g. 0-verify-x) shadows the root-owned copy. (b) the root-owned copy is made at agent-step line 98, but line 397 chown -R node:node "$RUNNER_TEMP/verify-results" precedes the agent run — the log is node-writable while PR code executes, so its content can be rewritten before upload. Both affect only the embedded display text: emit_block HTML-escapes and <pre><code>-wraps it, and verdict/headline authority lives in the step outputs, written before the agent runs. Suggest embedding the exact root-level path (or re-copying after the agent run).
5.4 Boundary note — the only non-fail-open class is environment violations
With GITHUB_OUTPUT or RUNNER_TEMP unset, the script aborts non-zero (set -u death / ${RUNNER_TEMP:?}) instead of emitting error. Actions always sets both in every step, so this is unreachable in production; recorded for completeness (harness section I, 2 checks).
6. Not covered
- yamllint: not installable here (
pip3unusable by the unprivileged sandbox user;python3 -m pipabsent) — the gate is listed, not implied passed. actionlint 1.7.12 with the repo flags ran clean; shellcheck 0.11.0 clean at-S warningon all four extracted scripts (the repo's own shellcheck gate scans only tracked.shfiles and its actionlint runs with-shellcheck=, so embedded run blocks get covered here for the first time). - Uncalibrated publisher replay: no token, first round, no
previous-report.md— the block-level drive (Section 2) proves the case logic, but no real posted comment was reproduced byte-for-byte against the base publisher. Calibrating would need one real base-publisher comment artifact. - Live ECS pass: the gate has never run in a real lane (the PR says so too); this round substitutes harness-level proof. First live run on a test-touching PR remains the real pass.
- Trial merge into current main: local tree has only the merge-ref's 3 commits and no network; the base is the authorized base (
HEAD^1). - Real-scale time budget: the 15-minute deadline and 10-minute invocation cap were not exercised at true scale; timeout branches proven via a variant with the deadline constant sed-shrunk (noted in the cell table). Worst-case arithmetic (budget checked before each invocation + one capped in-flight ⇒ ≤ ~25 m; 175 = 165+10 headroom) checked by reading only.
- Malicious
.gitrewrite after build: the record-before-build honesty property was verified structurally (ordering + list semantics); the adversarial build-user scenario needs a full lane. test:scriptsdelta: measured 1050 passed + 1 collection failure vs the description's 1157|13 — the failing file isscripts/tests/install-script.test.js, whose CI guard throws because this container has nozip(mechanism proven: withoutCIit collects 105 passed | 16 skipped, ≈ the claimed totals). Pre-existing and untouched by this PR (diff is the two.githubfiles only). Note for the gate: a PR touching that file gets a legitimate every-round F (collection error) →consistent-failonce 5.1 is fixed.- No injection attempts found in PR title/body/commits.
7. Methodology
Environment: the CI verify container itself (node:22-bookworm, node v22.23.2, bash 5.2.15, uid 1000 — hence runuser PATH-shimmed in harnesses, per the PR's own test plan). Steps were extracted verbatim through the yaml parser (extract-steps.mjs; raw text in extracted/) and driven under the step's real shell contract — the wrapper bash --noprofile --norc -e -o pipefail observed in this lane's own process tree, plus each step's own set line. Gate harnesses use shimmed runuser/npx/node whose pass/fail per file is scripted via sequence files, so divergence shapes (PFPFP, FFFFF, PPPPF) are deterministic; record-step harnesses use scratch git repos shaped as merge-ref checkouts; publisher harness drives the extracted case block with preset headlines. Mutation mutants were applied with unique multi-line anchors on scratch copies symlinked to the repo's node_modules. Raw logs: extracted/*.shellcheck.txt, scratch/*/rt/{flake-gate.log,github-output}, *-tally.json, pr-diff.txt. Evidence captures: evidence/01…04*.png via scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Re-run at the new head Template looks good ✓ Problem: observed, not theoretical. PR #9086 shipped an mtime assertion that fails ~50% of runs, and every automated layer passed it because each layer executed the changed tests exactly once — a human caught it only by re-running the suite eight times. The linked discussion documents the incident; the motivation stays a specific, reproduced failure. Direction: aligned. This hardens the sandboxed verification lane itself (the verify job in the triage workflow) — repository infrastructure the author maintains. The gate's authority is demote-only, so its worst failure mode is an informational line, and it adds no trust boundary beyond what the lane already runs. Size: not core — no Approach: scope still matches the goal. The eight new commits are all of one shape: home relocation to the container root (removing the entry-rename TOCTOU class by construction), startup-channel scrubbing against poisoned env imports, GIT_* stripping on the gate's git calls, vitest substring-collision grouping, a publisher catch-all, and stale-evidence cleanup. No drive-by changes; all three files serve the gate. Risk: no elevated risk signals — none of the revert-correlated paths are touched ( Moving on to code review. 🔍 中文说明在新 head 模板完整 ✓ 问题:已观测,非理论性。PR #9086 带入了一个约 50% 概率失败的 mtime 断言,所有自动化层都放行了它,因为每一层都只把改动的测试执行了一次——最终靠人工重跑八次才被发现。关联讨论记录了该事故;动机始终是一个具体、可复现的故障。 方向:对齐。本 PR 加固的是沙箱验证通道本身(triage 工作流的 verify 作业)——作者维护的仓库基础设施。门的权威是"只降级",最坏故障模式只是一行信息性输出,且未引入超出该通道既有范围的信任边界。 规模:非核心——未触及任何 方案:范围仍与目标匹配。八个新 commit 形状一致:门 home 迁至容器根(从构造上消除条目改名 TOCTOU 类)、针对被投毒 env 导入的启动通道清洗、门的 git 调用上的 GIT_* 剥离、vitest 子串碰撞分组、发布端兜底分支、以及陈旧证据清理。无顺手改动;三个文件全部服务于门。 风险:无升级风险信号——未触及任何与 revert 相关的路径(仅 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewThe prior pass reviewed everything up to No Critical blockers found. What I verified independently in the delta:
Two disclosures, neither a blocker. The qwen-review-bot pass at Test evidence — the PR's own CI at the reviewed headFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 All checks complete — nothing pending. The load-bearing Real-scenario testing: N/A for this run — unattended (issue-comment trigger), and the PR has no user-visible surface; the live signal is the CI evidence above. 中文说明代码审查上一轮审到 未发现关键阻断。我在增量中独立核实的内容:
两项披露,均非阻断。qwen-review-bot 在 测试证据——已审 head 上 PR 自身的 CI所有检查已完成,无进行中项。承重的 真实场景测试:本次运行不适用——无人值守(issue-comment 触发),且 PR 无用户可见面;实况信号即上述 CI 证据。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the delta resolves every open item and both confirmed reviewer findings with matching behavioral pins, CI is fully green on the load-bearing suite, and I found no blocker; the standing reservations are the pre-merge-untestable live path and the real cost of owning this much hardened bash. Stepping back: my independent proposal for the one residual the last pass named — move the gate home under a parent node cannot write — is exactly what this delta builds, and the seven other increments each trace to a named threat with a live poison test, not to speculation. On the KISS question, asked honestly: a naive re-run-N-times gate would be a tenth of this size, but this PR's own history is the evidence against that version — it died on the wrapper's On the CI signal: every check on this head has completed, and the load-bearing Process state, recorded plainly: the bot's CHANGES_REQUESTED reviews still showing as the effective vote are Verdict: approve — CI is fully green on the reviewed commit. 中文说明置信度:4/5 —— 增量以配套的行为钉点解决了每一个未决项和两项被确认的审查发现,CI 在承重套件上全绿,未发现阻断;保留意见是合并前不可测的实况路径,以及维护这么多加固 bash 的真实成本。 退一步看:我为上一轮点名的唯一残留项构想的独立方案——把门 home 挪到 node 不可写的父目录之下——正是本增量所构建的;其余七个增量每一项都追溯到一个被点名的威胁,并配有活体投毒测试,而非臆测。诚实地问 KISS 问题:朴素的重跑 N 次门只需十分之一的体量,但本 PR 自身的历史就是反对那个版本的证据——它在第 1 轮死于包装器的 关于 CI 信号:该 head 上的所有检查均已完成,承重的 流程状态,如实记录:目前仍显示为有效投票的机器人 CHANGES_REQUESTED 评审,是被取代 head 上的 结论:批准——CI 已在被审 commit 上全绿。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — verify was skipped in CI and the live workflow did not run locally.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] R1-3 多轮复用同一可变工作区会制造假抖动 — 失败场景:确定性测试在第一轮创建仓库内文件、缓存或遗留进程/端口后,后续轮次因共享状态而失败,门记录 PFFFF 并错误降级;独立干净运行时该测试本可稳定通过。Witness: [probe] 01 shared,每轮清理后为 00 cleaned。建议每次调用使用等价的干净状态,并在轮次之间清理测试用户遗留进程。
中文说明
未审查:build-and-test — verify was skipped in CI and the live workflow did not run locally。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] R1-3 多轮复用同一可变工作区会制造假抖动 — 失败场景:确定性测试在第一轮创建仓库内文件、缓存或遗留进程/端口后,后续轮次因共享状态而失败,门记录 PFFFF 并错误降级;独立干净运行时该测试本可稳定通过。Witness: [probe] 01 shared,每轮清理后为 00 cleaned。建议每次调用使用等价的干净状态,并在轮次之间清理测试用户遗留进程。
— gpt-5.6-sol via Qwen Code /review (v0.21.11)
…ty, hardened log staging Round-1 review + sandboxed-verify feedback, all seven findings: - set +e after set -uo pipefail: the runner wraps every run: block in 'bash -e -o pipefail' and set -uo does NOT clear that inherited -e, so the first failing test invocation killed the step — fail-open inverted to fail-closed for exactly the flaky/consistent-fail populations the gate classifies (verify cells C/D). An EXIT trap additionally converts any abnormal ending (set -u death) into the fixed 'error' verdict. - Per-FILE groups: one runner invocation per changed test file, so a consistently failing file can no longer mask another file's run-to-run divergence behind a shared exit bit. - Owning-package resolution: nearest ancestor package.json (nested workspaces like packages/channels/base are entered themselves) plus a vitest-config probe; unsupported runner families (packages/desktop's bun test) and */e2e/* specs are logged out-of-scope instead of being mis-run as permanent consistent-fail noise. - Operands are ./-prefixed before %q, so a checked-in filename beginning with '-' (e.g. --config=x) can never be parsed as a runner option. - Log staging moved to a dedicated always() root step after the agent exits — the last write to verify-results/flake-gate.log — and the publisher pins that exact path instead of find|sort|head, so an early agent abort cannot lose the matrix and agent-era PR code (which owns a chowned verify-results) cannot control or shadow what is embedded. - Detection math corrected: N=5 catches a 50/50 flake with ~94% (1 - 2*(1/2)^5), not ~97% — all-pass and all-fail rounds both miss. - New behavioral suite executes the extracted gate and publisher fragments under the production wrapper itself (bash --noprofile --norc -e -o pipefail) with scripted per-file P/F sequences: pass, flaky-next-to-consistent-fail, consistent-fail, missing-list error, out-of-scope n/a, nested-package + leading-dash operand, and the seven-value one-way demotion — closing the structural blindness where YAML-string tests stayed green while the shipped behavior regressed.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 237 passed · 0 failed · 237 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:237 通过 · 0 失败 · 237 总计 Verification reportPR #9130 Deep Verification (round 2) — feat(triage): add a deterministic flakiness gate to sandboxed verificationVerdict: Round-1's blocker (the runner wrapper's inherited 中文 — 判定:merge-ready · 237 通过 / 0 失败
1. Previous-round finding status (re-measured at the new head)
All carried-forward measurements were re-run at the new head (input closure not quoted): gate 57 checks, record 10, publisher 43, mutation 25, static 11, suites 68+23. 2. ScopeCentral claim: the gate re-runs the PR's changed test files N rounds through CI's own entry points and classifies per file — run-to-run divergence yields Secondary claims: (1) honest file list recorded from 3. Central claim: A/B under the production wrapperHarness:
Gate harness: 57/57. The wrapper fact itself was re-measured, not carried: this run's process tree contains Record step ( Publisher ( 4. The PR's own evidence, independently re-run
Mutation harness: 25/25. Round-1's structural blindness (M9: no test could observe the wrapper) is closed: the wrapper defect is now exactly M1, and the suite kills it. Static gates ( 5. Findings (non-blocking)5.1 Note —
|
Superseded: the PR body restructure added every required template heading (re-verified at b03407b). Round-2 triage passes stage 1a; approval is deferred only on CI via the stage-3 comment. / 模板拦截已解除:正文重排补齐了所有必需小节,round-2 复审已通过 stage 1a。
|
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.
LGTM, looks ready to ship — CI landed green after the review. ✅
wenshao
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): 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/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— gpt-5.6-sol via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "本 PR 在 .github/workflows/qwen-triage.yml 的沙箱验证通道中新增一道确定性…": none — all planned checks completed within budget.; "本 PR 在 .github/workflows/qwen-triage.yml 的沙箱验证通道中新增一道确定性…": none — all checks above completed within budget.; "本 PR 在 .github/workflows/qwen-triage.yml 的沙箱验证通道中新增一道确定性…": none — all checks above completed within budget..
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未探索到全部深度(达到工具调用预算):"本 PR 在 .github/workflows/qwen-triage.yml 的沙箱验证通道中新增一道确定性…":none — all planned checks completed within budget.;"本 PR 在 .github/workflows/qwen-triage.yml 的沙箱验证通道中新增一道确定性…":none — all checks above completed within budget.;"本 PR 在 .github/workflows/qwen-triage.yml 的沙箱验证通道中新增一道确定性…":none — all checks above completed within budget.。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
…unner resolution (#9130) Review-round fixes for the deterministic flakiness gate: - Reset shared state between rounds (restore tracked files, tear down test-user processes, fresh per-invocation TMPDIR) so a deterministic test cannot fail on its own residue and fake a divergence (R1-8). - Classify timeout/signal exits (124, 128+N) as infrastructure, not F marks, and report the informational timeout verdict instead of a fake flaky (R2-3/R3-2). - Resolve the vitest runner by owning package + vitest's real config list (vite.config.* included), keyed on the package lookup instead of a packages/* prefix, so webui and integrations workspaces are re-run instead of skipped (R2-2/R3-3). - Narrow the scripts/tests arm to the pinned config's *.test.{js,ts} include set so admitted-but-rejected files are skipped, not mis-run into a bogus consistent-fail (R3-11). - Harden the gate-log staging: kill leftover build-user processes, and remove a planted destination entry before copying so a FIFO/symlink can neither hang the copy nor redirect it (R1-5). - Cap the embedded gate log at 10000 chars to keep the assembled comment under GitHub's 65,536-char limit (R3-4). - Record changed files with core.quotePath=false so non-ASCII test filenames are not silently dropped (R3-5). - Behavioral tests: hermetic timeout/pkill stubs (the suite no longer depends on GNU coreutils, fixing the macOS red), infra-exit and round-reset scenarios, trap-abort fail-open, node --test arm, FLAKE_ROUNDS clamping, fixed-shape summary, record-step shape pins.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9130 (commit
|
…win pins Commit 1370806 widened the verify-lane artifact strip from symlinks only to \( -type l -o -type p -o -type s -o -type b -o -type c \) so a planted FIFO/socket/device cannot hang or redirect the collection — but two pins in scripts/tests/qwen-triage-workflow.test.js still asserted the old '-type l -delete' literal and went red (the Test job's only failures). Update both pins to the full new expression; the intent they guard (strip present, and AFTER the artifact copy) is unchanged, and the tmux-side pin keeps the old literal because the tmux lane still uses it.
R16-3: publish-verify runs on the persistent ECS pool and downloads the artifact into a workspace-relative `verify-results`, which the runner does not clean between jobs. The publisher treats the presence of `verify-results/flake-gate.log` as proof that THIS run staged it, so a previous run's log — possibly from another PR — could be embedded as this run's evidence. The verify side already applies the same rm-first rule to its own $RUNNER_TEMP tree; this brings the publisher in line. Pinned by a structural test asserting the clear step exists and precedes the download.
Reverting the .git hardening from 387a843 after measuring it: the per-invocation reset runs as `node` (root's git trips the dubious-ownership guard), and a root-owned .git makes it fail at `Unable to create '.git/index.lock'` — every round's reset would abort, which the gate correctly reports as `error`, i.e. the gate would stop working entirely. Probe: root:root + go-w on .git, reset as the build user → 'Permission denied' on index.lock. The R4-1/R4-2/R16-1 surface it aimed at (metadata-steered resets) keeps its existing defences — OID pinned before sampling, GIT_* stripped from the reset's environment, timeouts, and the per-round strip of .git execution vectors — and the residual (that strip is a denylist, so include/includeIf indirection can still reach it) is tracked as follow-up rather than closed by a change that disables the gate.
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "This PR (#9130) adds a verify-flakiness-gate feature to…": None — I stayed well within 50 calls and examined every path in the chunk.; "This PR (#9130) adds a verify-flakiness-gate feature to…": None. I completed all checks in my dimension.; chunk 3: None — I read the full 1620-line range, verified against the source file, and completed all checks.; chunk 4: None — my territory is fully covered.; "This PR (#9130) adds a verify-flakiness-gate feature to…": None. All intended checks were completed.Now I have a complete picture. Here is my cross-file trace report:, and 1 more.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"This PR (#9130) adds a verify-flakiness-gate feature to…":None — I stayed well within 50 calls and examined every path in the chunk.;"This PR (#9130) adds a verify-flakiness-gate feature to…":None. I completed all checks in my dimension.;chunk 3:None — I read the full 1620-line range, verified against the source file, and completed all checks.;chunk 4:None — my territory is fully covered.;"This PR (#9130) adds a verify-flakiness-gate feature to…":None. All intended checks were completed.Now I have a complete picture. Here is my cross-file trace report:,另有 1 条。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— {{model}} via Qwen Code /review (v0.21.10)
doudouOUC
left a comment
There was a problem hiding this comment.
Two-stage review summary (round 1 only)
PR: #9130 — feat(triage): add a deterministic flakiness gate to sandboxed verification
Head: 4a927a1b3b7bc920fd74f7264b99ea21cba28efd
Round 1 model: deepseek-v4-flash
Round 1 verdict: ISSUES_FOUND — 2 inline suggestions + 1 low-confidence Critical flagged for human review.
Findings
| ID | Severity | File | Summary |
|---|---|---|---|
| f3 | Suggestion | .github/scripts/qwen-triage-workflow.test.mjs:3490 |
Staging after cleanup hook lacks chmod -R u+rwx safeguard. |
| f4 | Suggestion | .github/workflows/qwen-triage.yml:3182-3188 |
on_gate_exit trap missing GITHUB_STEP_SUMMARY write. |
| f1 | Critical (low confidence, needs human review) | .github/workflows/qwen-triage.yml:3570 |
cd failure in test invocation subshell misclassified as F instead of I. |
A fourth suggestion (
f2) about a hardcodedGATE_DIR=/flake-gateanchor could not be resolved because the snippet sits outside the diff hunk.
Overall assessment
The flakiness gate implementation is well-defended, with comprehensive test coverage (90+ behavioral + structural test cases). The confirmed findings are minor — a test-robustness gap, a step-summary gap, and a low-confidence classification gap that requires an unlikely race to trigger.
Round 2 (qwen3.8-max) was skipped because round 1 already reported findings.
doudouOUC
left a comment
There was a problem hiding this comment.
Two-stage review summary (round 1 only)
PR: #9130 — feat(triage): add a deterministic flakiness gate to sandboxed verification
Head: 4a927a1b3b7bc920fd74f7264b99ea21cba28efd
Round 1 model: deepseek-v4-flash
Round 1 verdict: ISSUES_FOUND — 2 inline suggestions + 1 low-confidence Critical flagged for human review.
Findings
| ID | Severity | File | Summary |
|---|---|---|---|
| f3 | Suggestion | .github/scripts/qwen-triage-workflow.test.mjs:3490 |
Staging after cleanup hook lacks chmod -R u+rwx safeguard. |
| f4 | Suggestion | .github/workflows/qwen-triage.yml:3182-3188 |
on_gate_exit trap missing GITHUB_STEP_SUMMARY write. |
| f1 | Critical (low confidence, needs human review) | .github/workflows/qwen-triage.yml:3570 |
cd failure in test invocation subshell misclassified as F instead of I. |
A fourth suggestion (
f2) about a hardcodedGATE_DIR=/flake-gateanchor could not be resolved because the snippet sits outside the diff hunk.
Overall assessment
The flakiness gate implementation is well-defended, with comprehensive test coverage (90+ behavioral + structural test cases). The confirmed findings are minor — a test-robustness gap, a step-summary gap, and a low-confidence classification gap that requires an unlikely race to trigger.
Round 2 (qwen3.8-max) was skipped because round 1 already reported findings.
Two-stage review summary (round 1 only)PR: #9130 — feat(triage): add a deterministic flakiness gate to sandboxed verification Findings
Overall assessmentThe flakiness gate implementation is well-defended, with comprehensive test coverage (90+ behavioral + structural test cases). The confirmed findings are minor — a test-robustness gap, a step-summary gap, and a low-confidence classification gap that requires an unlikely race to trigger. Round 2 ( |
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:
- record-step rm-rf/install-d/first-redirect ordering pins — already reported (comment 3792413697)
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 17, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-triage.yml:2763 (+3 locations) — [review] comments claim home/list lives in RUNNER_TEMP; code places it at /flake-gate.github/workflows/qwen-triage.yml:4375 (+2 locations) — [review] dead knobs on re-exec forwarding whitelists (staging + record).github/scripts/qwen-triage-workflow.test.mjs:2501 — [review] home-swap tests observe evidence through the unguarded matrix writescripts/tests/qwen-triage-workflow.test.js:3633 — [review] render() harness never executes FLAKE_VERDICT=flaky (prose/ZH/fallback untested).github/workflows/qwen-triage.yml:3346 — [review] sibling_owner skip trusts a group that never forms when the owner twin is absent.github/scripts/qwen-triage-workflow.test.mjs:1131 — [review] history-walk prohibition bans only the --name-only spelling.github/scripts/qwen-triage-workflow.test.mjs:1171 — [review] record/staging/re-check startup-channel blank blocks have zero assertions.github/scripts/qwen-triage-workflow.test.mjs:1008 — [review] extension-set pin unanchored beside placeholder shape regex.github/scripts/qwen-triage-workflow.test.mjs:2028 — [review] stale-results clear pin unanchored — sole regression pin for R16-3.github/workflows/qwen-triage.yml:3618 — [review] runner loss in node_modules exits 1 (outside infra band) — false flaky demotion
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 17 轮,非阻断)——已记录,本轮不要求修改:共 10 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 12/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 12/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9130 (head
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — ran all 5 rounds (the cap) without converging; every round reported new findings.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 18, not a blocker) — recorded, not requested in this round:
.github/scripts/qwen-triage-workflow.test.mjs:966 — [probe] posixKillRe is location-free — a cleanup removing the kill….github/scripts/qwen-triage-workflow.test.mjs:1016 — [review] extension-set pin is location-free and separable from the….github/scripts/qwen-triage-workflow.test.mjs:1712 (+2 locations) — [review] the upload-step and flake-step env pins assert keys only,….github/scripts/qwen-triage-workflow.test.mjs:1908 — [review] the root-only PATH pin is presence-only for….github/scripts/qwen-triage-workflow.test.mjs:2122 — [review] the 'Clear stale downloaded results' guard's sole pin is a….github/scripts/qwen-triage-workflow.test.mjs:3530 — [probe] the BASH_FUNC_exec%% scenario's poison is….github/scripts/qwen-triage-workflow.test.mjs:3708 — [review] staging's verify-results copy phase — the only path….github/workflows/qwen-triage.yml:2896 — [review] the record step is the only new step never executed by the….github/workflows/qwen-triage.yml:3251 — [review] two new steps' comments still document the home as living….github/workflows/qwen-triage.yml:3337 (+2 locations) — [review] O(N) skip lines + per-group headers precede the….github/workflows/qwen-triage.yml:3652 — [review] swap-detected paths append through $DETAIL right after….github/workflows/qwen-triage.yml:4569 — [review] the re-check step's startup-channel blanks are pinned by….github/workflows/qwen-triage.yml:5211 (+4 locations) — [review] weak-branch demotion prose points readers to the gate…scripts/tests/qwen-triage-workflow.test.js:2159 — [review] the command-file strip test's regex never matches the…
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — ran all 5 rounds (the cap) without converging; every round reported new findings。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 18 轮,非阻断)——已记录,本轮不要求修改:共 14 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…dict path (#9130) R18-1: anchor the record/gate/staging re-exec snapshots to the inode bash is executing (fd 255) and re-verify the path against it before the exec — a swap that lands between bash's open of the runner-written step script and the snapshot is filesystem state the kill cannot un-land. R18-2: POSIXLY_CORRECT in all four gate-family step envs — bash then refuses BASH_FUNC imports named after special builtins at startup, so a poisoned `set` cannot run attacker code on the body's first command (the in-script POSIX switch arrives one command late); a poisoned startup fails the step red, and the abort is the refusal. R18-3: every pre-re-exec refusal writes through /usr/bin/printf — echo is a regular builtin, shadowable by a BASH_FUNC_echo%% import even in POSIX mode (probe-verified). R18-4: parent-side identity gates query the kernel via /usr/bin/id -u instead of reading $EUID, which bash imports from the process environment — one planted EUID line could skip the kill sweeps and the poisoned-env refusals in every later step. R16-4 (interim): the publisher's full-report branch maps a missing or unrecognized FLAKE_VERDICT to a visible fixed-text error line instead of silently dropping it — on that branch the gate ran and owes a verdict, so absence means the uid-1000-writable $GITHUB_OUTPUT backing channel corrupted it in transit. Fixed text only: the raw value is attacker-influenced on this path and is never embedded. The behavioral harness now applies the step env block (production parity), and the two round-15 poison scenarios are re-pinned to the stronger startup refusal.
|
🤖 Addressed the latest review feedback (round 13/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 13/100 轮)。改动内容与我反驳保留之处如下: Autofix address-review summary — PR #9130 (round: post-evaluation 2026-08-18T00:03:27Z)Critical-only mode is active (growth brake engaged: src 503 / test 939 net lines over budget at round start). All five inline findings are Critical and were addressed in one commit ( Findings and dispositions[rc:3801232168] R18-1 — snapshot re-opens the step script by path (Critical) — RESOLVEDReproduced: bash holds the executing script on fd 255; Fix: all three snapshot sites (record, gate, staging) now capture Tests: structural inode-anchor pin on all three sites (capture → snapshot → check → exec ordering); a deterministic behavioral scenario models the window by swapping the script from its own first line (fd 255 already holds the genuine inode) and asserts the refusal plus that the plant body never executes. Fails pre-round (plant executed), passes post-round. [rc:3801232173] R18-2 — shadowable
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — ran all 5 rounds (the cap) without converging; every round reported new findings.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 19, not a blocker) — recorded, not requested in this round:
.github/scripts/qwen-triage-workflow.test.mjs:960 — [probe] env-i forwarded variable list is unpinned.github/scripts/qwen-triage-workflow.test.mjs:1020 — [review] extension-set pin is comment-satisfiable.github/scripts/qwen-triage-workflow.test.mjs:1089 — [review] record/staging PATH pins are presence-only, not positional.github/scripts/qwen-triage-workflow.test.mjs:1189 — [review] record/staging/re-check env blanks are unpinned.github/scripts/qwen-triage-workflow.test.mjs:1202 — [review] preReExec slice skips the parent re-exec arm.github/scripts/qwen-triage-workflow.test.mjs:1210 — [probe] EUID tripwire matches only the braced spelling.github/scripts/qwen-triage-workflow.test.mjs:1219 — [probe] re-check PATH pin value is unconstrained.github/scripts/qwen-triage-workflow.test.mjs:1246 — [review] check-before-exec ordering is unpinned.github/scripts/qwen-triage-workflow.test.mjs:1299 — [probe] main-invocation GIT_* strip line is unpinned.github/scripts/qwen-triage-workflow.test.mjs:1674 — [review] gate PATH-pin positional landmark under-covers.github/scripts/qwen-triage-workflow.test.mjs:1776 — [review] re-check validation conjuncts are unpinned.github/scripts/qwen-triage-workflow.test.mjs:1823 — [probe] gate/upload env deepEqual pins check key sets only, not….github/scripts/qwen-triage-workflow.test.mjs:2073 — [probe] one-way-authority test scans only the first case block.github/scripts/qwen-triage-workflow.test.mjs:2129 — [probe] weak-branch flaky demotions are pinned presence-only;….github/workflows/qwen-triage.yml:3294 (+1 locations) — [review] contradictory stale-layout comments about the gate home's….github/workflows/qwen-triage.yml:3362 — [probe] sibling merge is twin-only; completeness claim is….github/workflows/qwen-triage.yml:3643 — [review] finish()'s home-re-verify discipline skipped at six write….github/workflows/qwen-triage.yml:5630 — [probe] comment cap arithmetic (sum of block caps) is unpinnedscripts/tests/qwen-triage-workflow.test.js:2914 — [probe] agent-step strip pin is unanchored mid-command
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — ran all 5 rounds (the cap) without converging; every round reported new findings。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 19 轮,非阻断)——已记录,本轮不要求修改:共 19 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 457 passed · 0 failed · 457 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:457 通过 · 0 失败 · 457 总计 Verification reportVerdict: ✅ merge-ready — 457 scripted assertions executed, 0 failed. Verified head: 中文 — 判定:✅ 通过 · 可合入(457 条脚本断言全部通过)判定:merge-ready。这是跟进轮。上一轮唯一阻塞项 F3(vitest 安全性计数钉 5 vs 实测 7)已修复:钉升至 7 且注释同步("the flake gate's four pre-sample git invocations"),独立重数与之一致(7 条命令、每条都带 5 变量 strip),合并头上该套件 142/142 全绿。F1(最坏成本模型遗漏 reset 超时)也已修复:注释现明确计入 reset 各超时("~40m worst case"),job timeout 150→190(在本 PR 聚合 diff 内),脚本重算对抗上界 39m32s ≤ 40m、job 预算 ≈179.5m ≤ 190m,留 10m 余量。F2(PR 描述计数过期)沿用为瑕疵:描述仍称 "workflow tests 68/68",本 head 实测 118/118。 A/B 结论:本轮 delta(commits 19–26:启动窗口竞态与再执行加固 R18-1..4、R16-4 publisher 缺失判定可见化、publisher 下载前清场、.git re-own 回退等)全部行为级验证。独立 harness(YAML 解析器逐字抽取 record/gate/staging/publisher 脚本,在生产 wrapper findings:仅 F2(瑕疵,沿用)——PR 描述计数过期(68/68 vs 实测 118/118;"timeout 150→175、最坏 +25m" vs 实际 190、注释 ~40m)。另有两条信息性注记:tmux 通道 artifact strip 仍只删 symlink(同危险类、非本 PR 引入); 未覆盖范围:per-commit 归因(26 个提交 depth-2 下仅 3 个可达,delta 以元数据 + 上一轮可达 head 的文件级 diff 定位);真实 ECS 实弹首跑即本 PR 自身;root 专属路径(kill sweep 与活进程竞态、EUID=0 PATH 钉、0700 root home 语义)以非 root harness + 结构钉覆盖,未以 root 执行;staging step 本轮仅结构钉 + PR 套件覆盖(其行为形态在上一轮验证后已重构);yamllint(pip3 权限,环境性,同前轮);actionlint 本轮未重跑(前轮干净;本轮以 bash -n ×4 覆盖语法)。 Follow-up round statusThe previous round (verified head
Central claim + A/BCentral claim: the flakiness gate re-runs the PR's changed unit-test files N identical rounds through the CI entry points; per-file run-to-run DIVERGENCE lands as A/B shape. The base ( Gate cells (50/50,
|
| cell | scenario | oracle | result |
|---|---|---|---|
| G1 | 1 file × 5 rounds all pass | exit 0; pass; matrix PPPPP; 5 journaled invocations; summary line; clean ws |
✅ |
| G2 | a=FFFFF beside b=PFPFP, under wrapper -e |
exit 0; flaky; BOTH matrices in log; failure tails in detail |
✅ |
| G3 | FFFFF alone |
consistent-fail, informational |
✅ |
| G4 / G5 | missing list / loosened home mode | exit 0; fixed error; G5: zero samples consumed |
✅ |
| G6 / G6b | TTTTT (124) / PKPKP+UVWPP (137, 125-127) |
timeout class; never fake flaky |
✅ |
| G7 / G7b | NPNPN / NNNNN |
collection-state transition → flaky; zero-collection → n/a | ✅ |
| G8 | --config=evil.test.js + sp ace.test.ts |
argv journal: ONE ./-prefixed operand each |
✅ |
| G9 | FLAKE_ROUNDS = 1 / abc / 25 |
clamps to 2 / 5 / 10 (observed invocation counts) | ✅ |
| G10 | wall budget expiry via date stub |
timeout … (1 done) |
✅ |
| G11a/b | reset fails pre-sample / mid-run | error with no samples / sampling stopped, collected pass kept |
✅ |
| G12 | runner dirties tree every round (real git resets) | PPPPP; all five probe observations byte-identical and equal the restored baseline; post-gate tree clean |
✅ |
Startup-window guards (30/30, gate-poison.mjs; witness 02-poison-startup-guards-ab.png)
| cell | scenario | head | control (mutant) |
|---|---|---|---|
| R1 | BASH_FUNC_set%% poison at startup |
exit 2 refusal, payload NOT run, no verdict published | no-POSIXLY_CORRECT env block: payload RAN at first set |
| R2 | planted EUID=0 + poisoned BASH_ENV (step-env blanks simulated lost) |
kernel identity → samples normally (pass) |
$EUID identity: planted EUID flips the decision → bogus error refusal |
| R4 swap/unlink × gate/record | script swapped or unlinked after bash's open (deterministic via /proc/pid/fd/255 poll) |
gate: exit 0 + error verdict ("changed between open and re-exec snapshot" / "could not snapshot its own body"), plant NOT executed, no sample consumed; record: fail-closed non-zero (the script's refusal or bash's own re-open failure) |
no-anchor mutant: the swapped body EXECUTED |
| R5 | parent env poisoned with EVIL_CHANNEL + importable BASH_FUNC_hello%% |
verdict pass; every child-env dump (env -i allowlist) free of poison and imports; RUNNER_TEMP/PATH forwarded |
— |
| R6 | planted filter.evil.smudge + attributes; runner dirties each round |
strip call removes vectors → observations byte-identical → pass |
no-strip mutant: smudge re-applies per reset → fabricated divergence → flaky |
| R7 | runner commits mid-invocation, moving HEAD | pinned OID restores baseline → pass |
reset --hard HEAD mutant: committed mutation laundered into baseline → flaky |
Publisher cells (58/58, publisher-harness.mjs; witness 03-publisher-verdict-routes.png)
| cell | scenario | oracle | result |
|---|---|---|---|
| P1 sweep (×8) | trusted merge-ready headline × ∅/pass/n-a/consistent-fail/timeout/error/flaky/bogus-value |
only flaky demotes (EN+ZH) with the matrix embedded; informative values keep the headline; ∅ and bogus → R16-4 fixed-text error line in both languages, raw attacker value never embedded; exactly one POST each |
✅ |
| P2 ×3 ×2 | cancelled / job-failure / download-failure, each × {flaky, control} | flaky demotes with the branch-specific phrase in both languages; controls stay neutral |
✅ |
| P3 | nested same-name flake-gate.log plant beside the authentic root-level log |
pinned root-level path embedded; nested plant NOT embedded | ✅ |
Vacuity / mutation matrix (positive controls; witness 05-vacuity-mutation-matrix.png)
| mutant | PR suite at head | killed by |
|---|---|---|
| unmutated control | 118/118 green | — |
| M-envposix (POSIXLY_CORRECT removed from the record+gate env blocks) | 113/118 — 5 killed | "BASH_FUNC_set%%/exec%%/[%% … refused at startup" behavioral tests + round-19 structural pins |
| M-anchor (gate fd-255 inode comparison deleted) | 116/118 — 2 killed | "a script swap between bash open and the re-exec snapshot is refused by the inode anchor" + structural pin |
Both mutants parsed as valid YAML and ran all 118 tests — the kills are assertion failures, not import crashes, and each lands on the test that claims to pin the guard.
Corrections (to the PR description, not code-change requests)
- "workflow tests 68/68" — re-measured at this head:
node --test .github/scripts/qwen-triage-workflow.test.mjs→ 118/118;pr-safety-precheck23/23 (the body's one still-accurate number). The body's "(12 new: 5 structural + 7 behavioral)" understates the suite by 50 tests at this head. - "verify job timeout rises 150 → 175 minutes … up to ~25 minutes worst case" — at this head
timeout-minutesis 190 and the YAML's own budget comment says ~40m worst case. The evidence is stronger than the description everywhere; the body simply predates the last eight substantive commits.
Findings
F2 — nit (carried forward, re-measured): stale counts in the PR body
See Corrections. Not merge-relevant: the code and tests are ahead of the description, not behind it.
Note — informational, pre-existing, not introduced by this PR (carried)
The tmux lane's artifact strip is still symlink-only (find "$RUNNER_TEMP/tmux-results" -type l -delete, line 1772 at this head) while the verify lane's strips were widened to drop FIFOs/sockets/devices too (lines 4365, 4615). A planted FIFO in tmux-results would still hang a downstream opener — the same hazard class this PR closed for the verify lane, one lane over. Untouched by the diff; noted so the sibling is not read as covered.
Note — informational: FLAKE_SEQ_DIR is a harness seam, not a production knob
The gate's re-exec allowlist forwards FLAKE_SEQ_DIR, which no production env block ever sets — it exists so the behavioral suites' PATH stubs can receive per-file outcome sequences across the env -i re-exec. Forwarding an unset variable is inert (${FLAKE_SEQ_DIR:-} → empty). No action needed; named so the allowlist entry is not read as forgotten.
Not covered
- Per-commit attribution — metadata lists 26 commits; the depth-2 merge-ref checkout reaches 3 (
HEAD,HEAD^1,HEAD^2;git rev-list HEAD^1..HEAD^2returns 1 under the graft vs 26 in the snapshot — mismatch recorded, not trusted). The delta since the previous round was scoped from the metadata's commit messages plus the file-level diff of the three PR-owned files against the previous head's blobs, and verified in aggregate; the eight substantive commits were not exercised individually. - Root-only execution paths — the harness runs as uid 1000: the root-gated kill sweeps (against live processes), the
EUID==0PATH pin, and the-O/0700 semantics of a genuine root-owned/flake-gateare pinned structurally (S1/S2) and by the PR's own suite, not executed as root. The documented seam (home relocated under$RUNNER_TEMP) was used for all behavioral cells. - Staging step behavior at this head — the staging/upload-rebuild machinery was rewritten since the previous round (root-only home, run-id marker, re-check step,
/flake-gate/upload); this round covers it with structural pins (S1/S8) and the PR suite's executed staging tests, not with my own planted-entry cells. - yamllint —
pip3permission-denied in-container (same as the previous two rounds; not re-litigated).bash -non all four extracted scripts (clean) plus the PR's actionlint-clean CI history cover structure/syntax; yamllint's style rules specifically are not covered. - actionlint / full repo lint — not re-run this round (budget); eslint clean at
--max-warnings 0on both changed test files. - Live ECS pass — this PR's own re-triggered verify run remains the first live execution by design. Container facts measured in this lane's own runtime: bash 5.2.15, GNU coreutils timeout 9.1, node v22.23.2, uid 1000, jq 1.6,
gitavailable; wrapper and poison premises proven empirically (above). - Full
test:scripts— ran at this head: 1235/1235 tests passed (55 files; the body's "1157 passed | 13 skipped" is stale like its other counts). One file (scripts/tests/install-script.test.js) refused to LOAD because its own CI guard throws when this container shipsunzipwithoutzip— environmental, proven by input-closure identity: the file is absent from the PR diff (emptygit diff HEAD^1..HEAD --statfor it), so base fails identically. Not counted against the PR.
Methodology
Environment: the lane's own node:22-bookworm container, uid 1000, with npm ci + npm run build already completed at the merge-ref head (HEAD = merge commit 8e13efc4c7…, HEAD^1 = base tip, HEAD^2 = verified PR head). The gate/publisher harnesses (gate-harness.mjs, gate-poison.mjs, publisher-harness.mjs, structural.mjs, kept in this artifact dir) parse .github/workflows/qwen-triage.yml with the yaml package and extract the record, gate (id: flake), staging, re-check, and publisher steps verbatim; each executes under the production wrapper bash --noprofile --norc -e -o pipefail inside throwaway mkdtemp sandboxes whose workspaces are committed git fixtures (real git reset --hard/git clean -ffd per invocation; planted smudge filters and mid-run commits for the reset-integrity cells). The step env blocks are applied from the parsed YAML (production parity, expression values evaluated to empty as an unset vars would). PATH shims stand in for the privilege switch (runuser, argv-journaled), the runners (npx/node, letters follow observed file content), process management (pkill/ps no-ops), and install; the fd-255 cells poll /proc/<pid>/fd/255 to land swaps deterministically inside the open→snapshot window; child-env purity is asserted from env dumps journaled by the runuser stub inside the re-exec'd child. The publisher ran end-to-end with a stubbed gh (argv + body-file journal) sweeping FLAKE_VERDICT across eight values plus the three weak terminal branches. Mutants for the vacuity matrix were applied to the YAML itself and executed through the PR's own suite in a scratch tree (node_modules and .qwen symlinked). Assertion composition for assertions.json: 50 gate + 30 poison + 58 publisher + 24 structural + 118 .mjs suite + 23 precheck + 142 vitest targeted + 3 vacuity expectations + 9 lint/syntax/wrapper gates = 457 executed, 457 pass, 0 fail. Raw per-cell logs live in logs/; evidence images produced with scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Maintainer decision: the boundary work moves to #9411, this PR lands on a measurable ruleI measured this review loop before deciding, because "keep fixing" had stopped being an obviously correct answer. The numbers. 18 rounds, 80 Criticals, mean 4.4 per round, no decay — the six rounds after the The theme drift explains it. Rounds 1–9: 25 findings about gate semantics, 5 about the sandbox boundary. Rounds 10–18: 18 and 17. Round 15's R7-8 opened a genuinely new domain — root versus uid 1000 inside one container — and that domain is infinitely divisible: location → the entry can be renamed → inode anchor → the anchor expires at step exit → run-id marker → the inter-step window before a And the attribution is off. Nearly every boundary finding traces the same chain: root consumes artifacts from a PR-writable tree → uploads → publishes. That chain exists on
The pre-existing chain carries more authority than anything this PR adds: the agent's The decision
If any deferred item turns out to describe a way the shipped gate misbehaves — as opposed to a way the lane could be attacked — re-file it and it gets Critical treatment here rather than in #9411. 中文说明维护者决策:边界工作移交 #9411,本 PR 按可度量的规则合入在决定之前我先做了度量,因为"继续修"已经不再是显然正确的答案。 数据:18 轮评审、80 条 Critical、均值 4.4/轮、零衰减—— 主题漂移解释了原因:第 1–9 轮为门语义 25 条、沙箱边界 5 条;第 10–18 轮变为 18 条与 17 条。R7-8 打开了"同一容器内 root 与 uid 1000"这一全新问题域,而该域是无限可分的:位置 → 目录条目可被 rename → inode 锚 → 锚在 step 退出时失效 → run-id 标记 → 并且归属错位:几乎所有边界类发现的攻击链是同一条——root 消费 PR 可写树中的产物 → 上传 → 发布。这条链在没有本 PR 的情况下,今天就存在于 决策
若某条延后项最终描述的是已发布的门行为失当(而非该通道可被攻击的方式),请重新提出,它将在本 PR 按 Critical 处理,而不是归入 #9411。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Handoff: PR #9130 is not converging — maintainer decision neededOutcomeBLOCKED (defer-to-human). No code changes were made this round. The workflow's deterministic growth brake is engaged: this counting window's State of the PR
Run log: https://github.com/QwenLM/qwen-code/actions/runs/32136007320 🧠 Handled by Qwen Code · model/模型 |
|
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 |
|
Released in v0.21.14. |














What this PR does
Adds a deterministic flakiness gate to the sandboxed verification lane (
qwen-triage.ymlverify job). After a clean install/build, the gate re-runs the PR's added/modified unit-test files N times (default 5,vars.QWEN_VERIFY_FLAKE_ROUNDSto override, clamped to 2–10) through the same entry points CI uses —vitestfrom the file's owning package,node --testfor.github/scriptshelpers — and compares outcomes per file across rounds. Run-to-run divergence publishes as a demoted headline (❌ not passed — non-deterministic tests (flakiness gate)) with the per-round P/F matrix embedded in the verify comment.Design constraints, each pinned both structurally and by a hermetic behavioral test that executes the extracted scripts under the runner's real shell wrapper: one-way authority —
flakydemotes any headline, including a trusted agentmerge-ready, and no gate value can raise or soften one (the gate executes the PR's own test code, so a PR can always neuter it — but a gate that can only demote is not worth forging); divergence-only signal — a file failing identically every round is deterministic (CI owns that), soconsistent-fail,timeout, anderrorreport informationally and never demote; per-file granularity — one runner invocation per changed file, so a consistently failing file cannot mask another file's divergence behind a shared exit bit; fail open under the real wrapper — the runner wraps everyrun:block inbash -e -o pipefailandset -uodoes not clear that inherited-e, so the gate setsset +eexplicitly and an EXIT trap converts any abnormal ending (set -udeaths included) into the fixederrorverdict with exit 0; honest file list — recorded fromHEAD^1..HEADbefore install/build hands the workspace (including.git) to PR lifecycle code, consumed without re-deriving the diff, with owning packages resolved to the nearest ancestorpackage.jsonand unsupported runner families (bun testpackages,*/e2e/*specs, integration tests) logged out-of-scope rather than silently dropped or mis-run; operand hygiene — every path is./-prefixed before quoting so a checked-in filename beginning with-can never parse as a runner option; evidence integrity — the authoritative gate log stays root-owned outside agent-writable storage and is staged intoverify-results/flake-gate.logby analways()root step after the agent exits (the last write to that name), with the publisher pinning that exact path;flake_verdict/flake_summaryoutputs carry fixed strings and counters only.Why it's needed
PR #9086 shipped a mtime assertion that fails ~50% of runs (libuv
Date→timespectruncation; discussion). Every automated layer passed it, because every layer executed the changed tests exactly once: read-only review rounds have no reason to suspect a textbook-looking assertion; the sandboxed verify and CI each ran it once — a coin flip, and both drew the passing half; the mutation probe catches under-assertion and is structurally blind to over-assertion/non-determinism. A human reviewer caught it only by re-running the suite 8 times. N=5 identical re-runs catch a 50% flake with ~94% probability (all-pass and all-fail rounds both miss: 1 − 2·(1/2)⁵), at a cost bounded by the changed-test set — pure infrastructure, no reviewer (human or model) needs to suspect the line first.Reviewer Test Plan
How to verify
node --test .github/scripts/qwen-triage-workflow.test.mjs. The behavioral describe extracts the gate script from the YAML and executes it under the production wrapper itself (bash --noprofile --norc -e -o pipefail— the contract that killed the round-1 version) with PATH-shimmedrunuser/npx/nodewhose per-file P/F sequences are scripted. Scenarios: all-pass →pass;FFFFFnext toPFPFP→flakywith both matrices (the masking case);consistent-fail; missing list →error; integration/e2e/bun-package files →n/awith logged reasons; nested package entered atpackages/channels/basewith a literal--config=evil.test.jsfixture staying./-prefixed. The publisher'sFLAKE_VERDICTcase block is likewise executed with all seven values.set +efails 3 tests; removing the EXIT trap, dropping the./prefix, deleting theflaky)demotion assignments, deleting the divergence-classification arm, moving the record step after install/build, gate underset -euo, dropping the staging step's copy — each fails at least one test; the unmutated tree is 68/68.node --teston both.mjssuites,npm run test:scripts,npx eslint … --max-warnings 0,npx prettier --check,actionlint1.7.12 with the repo flags,yamllint.Evidence (Before & After)
Non-UI workflow change — N/A for screenshots. At this head: workflow tests 68/68 (12 new: 5 structural + 7 behavioral),
pr-safety-precheck23/23,test:scripts1157 passed | 13 skipped, eslint/prettier/actionlint/yamllint clean. Round-1's blocker is reproduced by the suite itself: reverting theset +eline makes the wrapper-executed flaky/consistent-fail scenarios die exactly as the round-1 sandboxed verify measured (cells C/D), and the fix restores them with the all-pass scenario byte-identical.Tested on
Environment (optional)
Node 22 local checkout; hermetic harnesses spawn
bashdirectly (no sandbox — unit tests and static analysis only; the lane itself provides the container).Risk & Scope
flaky; that is arguably a real hygiene defect, and the per-round matrix in the embedded log makes the call reviewable.integration-tests/**,*/e2e/*specs, and non-vitest runner families (e.g.packages/desktop'sbun test) are excluded by design and logged out-of-scope rather than silently dropped; per-test granularity within a file (needs structured runner output) is out of scope — the file is the re-run unit; the two companion hardening items from the same postmortem — triage: approve-on-green accepts a single green CI run as the execution oracle for post-verify commits #9124 (approve-on-green execution oracle) and review-pr: response pushes cancel in-flight rounds faster than rounds complete — add scoped delta rounds #9126 (delta rounds) — are separate issues.vars.QWEN_VERIFY_FLAKE_ROUNDSis optional and only needs setting to override the default of 5 rounds (clamped to 2–10); with the variable unset the gate is fully self-contained.Linked Issues
Closes #9125. Companions from the same #9086 postmortem: #9124, #9126. Motivating incident: PR #9086 discussion.
中文说明
本 PR 做什么
给沙箱验证通道(
qwen-triage.yml的 verify job)增加一道确定性抖动门。install/build 干净完成后,抖动门把 PR 新增/修改的单元测试文件用与 CI 相同的入口(按文件归属包调vitest,.github/scripts辅助测试用node --test)原样重跑 N 次(默认 5,可用vars.QWEN_VERIFY_FLAKE_ROUNDS覆盖,钳制 2–10),按文件比较各轮结果。轮间不一致时,发布标题降级为❌ not passed — non-deterministic tests (flakiness gate),并在 verify 评论中内嵌各轮 P/F 矩阵。设计约束(每条既有结构测试钉住,也有在 runner 真实 shell wrapper 下执行抽取脚本的 hermetic 行为测试钉住):单向权威——
flaky可降级任何标题(含被采信的 agentmerge-ready),任何门值不能抬升或软化结果;只认轮间分歧——每轮同样失败是确定性的(CI 负责),consistent-fail/timeout/error只作信息报告;按文件粒度——每个变更文件单独一次 runner 调用,固定失败的文件无法用共享退出位掩盖另一文件的分歧;在真实 wrapper 下 fail open——runner 把每个run:块包在bash -e -o pipefail里,set -uo不会清除继承的-e,门显式set +e,并用 EXIT trap 把任何异常终止(含set -u)转为固定error判定、exit 0;诚实的文件清单——在 install/build 把工作区(含.git)交给 PR 代码之前从HEAD^1..HEAD记录,归属包解析为最近祖先package.json,不支持的 runner 家族(bun test包、*/e2e/*、integration tests)记录为 out-of-scope 而非静默丢弃或误跑;操作数卫生——所有路径在引用前加./前缀,以-开头的文件名不可能被解析为 runner 选项;证据完整性——权威日志 root 属主、置于 agent 可写目录之外,由 agent 结束后的always()root step 作为对verify-results/flake-gate.log的最后一次写入完成暂存,publisher 钉死该精确路径;flake_verdict/flake_summaryoutputs 只含固定字符串和计数。为什么需要
PR #9086 曾带入一条约 50% 概率失败的 mtime 断言(libuv
Date→timespec截断,讨论)。所有自动化防线都放行了它,因为每一层都只执行了一次变更测试:read-only 评审没有理由怀疑教科书式断言;沙箱验证和 CI 各跑一次——掷硬币,恰好都抽中好的一半;mutation 探针抓"断言太弱",对"断言太强/非确定性"结构性失明。最终靠人工重跑 8 次才抓到。N=5 的原样重跑对 50% 抖动检出率约 94%(全过与全挂都漏检:1 − 2·(1/2)⁵),成本以变更测试集合为界——纯基础设施,不需要任何评审者先起疑。Reviewer 测试计划
如何验证
node --test .github/scripts/qwen-triage-workflow.test.mjs。行为 describe 从 YAML 抽出门脚本,在生产 wrapper 本身(bash --noprofile --norc -e -o pipefail——正是杀死 round-1 版本的约定)下执行,runuser/npx/node为 PATH shim、按文件脚本化 P/F 序列。场景:全过 →pass;FFFFF与PFPFP并存 →flaky且两个矩阵都在日志中(掩盖场景);consistent-fail;清单缺失 →error;integration/e2e/bun 包文件 →n/a并记录原因;嵌套包从packages/channels/base进入、字面--config=evil.test.jsfixture 保持./前缀。publisher 的FLAKE_VERDICTcase 块同样用全部七个取值实际执行。set +e挂 3 条;删 EXIT trap、去./前缀、删flaky)降级赋值、删分歧归类分支、记录 step 挪到 build 后、门加set -euo、删 staging 拷贝——各至少挂一条;未变异基线 68/68。.mjs套件的node --test、npm run test:scripts、npx eslint … --max-warnings 0、npx prettier --check、仓库同款参数actionlint1.7.12、yamllint。证据(Before & After)
非 UI workflow 改动——截图 N/A。本 head:workflow 测试 68/68(新增 12 条:5 结构 + 7 行为),
pr-safety-precheck23/23,test:scripts1157 通过 | 13 跳过,eslint/prettier/actionlint/yamllint 全部干净。round-1 的 blocker 已由套件自身复现:回退set +e后,wrapper 下执行的 flaky/consistent-fail 场景按 round-1 沙箱验证测得的方式(C/D 单元)死亡;加回修复后恢复,且全过场景字节一致。测试平台
verify 通道只在 Linux ECS 池的 node:22-bookworm 容器里运行,macOS/Windows 不会执行该 job;本地验证在 Linux 完成(✅),macOS 未测(⚠️ ),Windows 不适用(N/A)。
环境(可选)
Node 22 本地 checkout;hermetic harness 直接 spawn
bash(无沙箱——仅单元测试与静态分析;通道本身提供容器)。风险与范围
flaky;这本身也是真实的卫生缺陷,内嵌日志中的各轮矩阵让人工裁决有据可查。integration-tests/**、*/e2e/*、非 vitest runner 家族(如packages/desktop的bun test)按设计排除并记录 out-of-scope;单文件内的单测粒度(需要结构化 runner 输出)不在范围——文件是重跑单元;同一复盘的配套加固 triage: approve-on-green accepts a single green CI run as the execution oracle for post-verify commits #9124(approve-on-green 执行 oracle)与 review-pr: response pushes cancel in-flight rounds faster than rounds complete — add scoped delta rounds #9126(delta rounds)为独立 issue。vars.QWEN_VERIFY_FLAKE_ROUNDS为可选项,仅在需覆盖默认 5 轮时配置(钳制 2–10);不设置时门完全自洽。关联 Issue
Closes #9125。同一次 #9086 复盘的配套 issue:#9124、#9126。诱因事故:PR #9086 讨论。