mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
Shaojin Wen
a268b7d175
* ci: restore the post-merge push trigger on main
`ci.yml` dropped its `push` trigger on the premise that the merge queue validates the merged tree before it lands. No merge queue is enabled — no queue run since 2026-07-02 — so nothing validated that tree on either side of the merge: `main` had no gate before a merge, and no check faster than the ~40-minute E2E after one.
Accept `push` on `main` in the `test` job alone. Every other job keeps its existing gate, so a post-merge run is exactly Classify PR + Test, and a nightly still resolves to exactly the two platform lanes. The expensive, PR-shaped steps inside `test` — the no-AK integration gate, the checkout-head verifier and the test reporter — already carry their own pull_request / merge_group gates, so the push lane is lint, static analysis and unit tests.
Route the lane to the ECS pool: a push to `main` is the most trusted context there is, and Test is the ten-plus-minute job that should not spend a scarce hosted Linux runner on every merge. classify_pr's own `runs-on` keeps no push arm by design — it is the canonical association-routing text that sdk-java.yml and serve-ab.yml mirror, and a push carries no author association to route on — so the two halves disagree on push deliberately; a new routing test pins that asymmetry so a later "drift fix" has to read the reason first.
* ci: address review findings on the post-merge push lane
Eight findings from the review pass, all confirmed against the tree.
R1-1 (Critical): the `test` job's checkout `ref:` fell through to `github.ref` on push — `refs/heads/main` resolved at fetch time, a moving tip — while the check run attaches to `github.sha`. The lane could validate a tree it did not report on, and nothing detected it: the checkout-head verifier is gated to pull_request / merge_group and its ancestor check passes for a newer tip regardless. Regressions would still be caught (the tested tree is a descendant), but attribution breaks — red on a clean commit, autofix filed against the wrong merge. Pin push runs to `github.sha`, mirroring the merge_group arm; `e2e.yml` gets the same guarantee from actions/checkout's default. The five other checkout copies sit in jobs that exclude push.
R1-5: the concurrency group was keyed on ref alone, so a post-merge run shared `Qwen Code CI-refs/heads/main` with the nightly (60-minute lanes, neither cancelling on main) and with any PR from a fork branch named literally `refs/heads/main` — a legal ref name that makes the group byte-identical while that run cancels in progress. Scope the group by event, as `e2e.yml` already does.
R1-6: `WORKFLOW_SIZE_BASE_SHA` was empty on push, so both size enforcers failed closed there while PRs stayed green on leniency — the next stale baseline would redden every merge with an error naming a PR that does not exist. Add `github.event.before`; `github.sha` stays rejected because workflow_dispatch would compare a file against itself.
R1-4: the coverage upload ran on push although its only consumer, post_coverage_comment, is gated to pull_request and nothing else reads the artifact.
R1-2: the push lane's YAML wiring had no test witness — the routing tests drive pick_runner's shell directly and stay green if the trigger or either job gate loses its push arm. Add assertions covering the trigger, both accepting jobs, the exclusivity of every other job, and the three fixes above. Each is mutation-checked: removing any one fix reddens exactly one test.
R1-3, R1-7 and R1-8 are three comments the trigger falsified: the failure watcher's "SCHEDULED run only" header, af-025's "ci.yml has no push trigger" premise, and this PR's own "only `test` accepts this trigger", which contradicted the classify_pr arm added in the same diff.
The added prose put ci.yml over its size baseline; trimmed back under the allowance rather than raising the baseline.
* ci: harden the push-lane guards after round-2 review
Five follow-up findings, all on the guards added in the previous commit rather than on the lane itself.
R1-2 (fix-induced): `test`'s push admission was pinned only negatively — no `!= 'push'` — so rewriting `test.if` into allowlist form drops push while every guard stays green: the exclusivity helper skips `test` because it is an expected push job, and the routing tests inject EVENT_NAME=push straight into pick_runner's shell, bypassing the job gate entirely. On a real merge the run would conclude `success` with the lane silently gone, and the failure watcher — gated on `conclusion == 'failure'` — would file nothing. Require the gate to stay exclusion-shaped.
R2-1: the push exclusivity loop was a hand-copy of the nightly one and the two had already diverged — the mirror's allowlist gained `schedule` while the original never gained `push` — so a push-only job would fail the nightly assertion with a message naming the wrong event. Replaced both with one parameterized helper, which also restores the `not.toBe('')` assertion the copy dropped.
R2-2: the concurrency assertion pinned only the added `github.event_name` token. Flipping `cancel-in-progress` to `true` was measured invisible to all 1854 test:scripts tests, while consecutive merges would then cancel each other — the history this repo already recorded once, when 67 of 100 push runs were cancelled and only 25 reported. Pin all three components, the way e2e-workflow.test.js pins its own.
R2-3: the group's pull_request half was still keyed on `head_ref` alone, so same-named branches across forks — and a fork's default branch is `main`, the commonest head_ref there is — share one group and cancel each other's in-progress runs. Add the source-repo term, matching security-checks.yml and serve-ab.yml. Supersede within a PR is preserved: one branch backs at most one open PR per fork.
R2-4: the trigger was pinned by `branches` only; a sibling `tags:` key would widen the lane while the suite stayed green. On a tag-creation push `github.event.before` is all zeros, so WORKFLOW_SIZE_BASE_SHA is unresolvable and both size enforcers take their strict arm, while the failure watcher's `head_branch == 'main'` filter files nothing — a silent red on release day. Pin the whole trigger object.
Mutation-checked: each of the five reddens exactly the intended assertion, and the extracted helper now reports the correct event for a push-only job (verified by injecting a probe job — the nightly assertion passes, the push one fails naming push).
* ci: decide push-lane gates by evaluation, pin them by literal
Round 3 raised seven findings, all Suggestion, and they are all instances of one root cause the review itself named: the guards asserted on substrings, and a substring cannot see a connective. Rather than adding one more substring per mutation, this replaces the two mechanisms behind them.
Gate reachability is now decided by EVALUATING each job's `if` for the event instead of looking for tokens in it — the substitute-then-evaluate technique `.github/scripts/ci-runner-routing.test.mjs` already uses on `runs-on`. That closes R2-1's connective blindness (an `&&`→`||` flip inside an excluded job; an allowlisted equality paired with an event-neutral disjunct under `||`) without the recursive boolean parser the suggestion proposed: a hand-written expression parser is a new fail-open surface of its own, whereas evaluation computes the answer rather than pattern-matching the question. Any context term the substitutions do not model now throws instead of reading as excluded. The helper also asserts the positive direction — that the jobs which must run on an event do — which the token version never did, and which is precisely the "a lane nobody is told about" failure this file's header says it exists to prevent.
Every security-relevant gate is pinned as a whole literal rather than by substring: `classify_pr.if`, `test.if`, the checkout `ref:` expression, the coverage step's `if`, and both halves of the concurrency block. One shape closes R3-3, R1-2, R3-4 and R2-2 together — connector flips, added exclusions, deleted clauses and reordering all change the literal.
R3-2: the trigger comment claimed the test reporter is restricted to pull_request / merge_group. It carries no event gate at all; it is excluded by a same-repo clause that also keeps fork PRs out, so normalising it to the documented allowlist would run dorny/test-reporter on fork events that lack `checks: write`. Reworded per step, with that trap named.
R3-1: bump `.size-baseline` for ci.yml to 85336. The previous two rounds trimmed comments to stay under the allowance, which left 62 bytes of headroom and charged the next PR for growth this one introduced. The baseline file's own header says growth is deliberate and asks for the number to be bumped with a reason — the growth here is the per-finding rationale the review asked for, so it is recorded rather than squeezed out.
Mutation matrix across all three rounds: 22 mutations, none escaping — the seven this round probed as surviving now redden, and the twelve from rounds 1 and 2 still do.