From 922dbdbf3ae92f0d24341b0499988d5914e8ea10 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 23:56:56 +0000 Subject: [PATCH] =?UTF-8?q?fix(ci):=20=E4=B8=8B=E6=B8=B8=20job=20=E6=94=B9?= =?UTF-8?q?=E7=94=A8=E6=98=BE=E5=BC=8F=E7=8A=B6=E6=80=81=E5=87=BD=E6=95=B0?= =?UTF-8?q?,`filter`=20=E5=A4=B1=E8=B4=A5=E4=B8=8D=E5=86=8D=E9=9D=99?= =?UTF-8?q?=E9=BB=98=E8=B7=B3=E8=BF=87=E4=B8=83=E4=B8=AA=E6=A0=B8=E5=BF=83?= =?UTF-8?q?=E9=97=B8=E9=97=A8=20(#4928)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ci.yml` 的 7 个下游 job 原本都是 `if: needs.filter.outputs. == 'true'`。 `if:` 里不出现状态函数时,GitHub 会隐式包一层 `success()`,于是 `filter` 一旦 失败(checkout 抖动、dorny/paths-filter 故障、10 分钟超时),test / temporal-conformance / dogfood / dogfood-verify / build-core / build-docs / console-pin 会被一起跳过 —— 而 skipped 在分支保护里算通过。结果是零测试运行、 全绿可合、没有任何红色信号。 七处统一改成「只有过滤器明确说 false 才跳」: if: ${{ !cancelled() && needs.filter.outputs. != 'false' }} 这与 filter 自身 outputs 上 `|| 'true'` 的取舍完全一致 —— 存疑就全跑。每个 job 保留自己的输出名(core / docs / console)。 聚合闸门补上同一层论证:test-gate 与 dogfood-gate 把 `filter` 纳入 needs, 当某条腿是 skipped 而 `filter` 未成功时判红,不再无条件接受 skipped。这条 分支在新 `if:` 下不可达,留作不变量的常驻断言 —— 它守的失败模式是静默的。 `cancelled` 两侧仍然放行(#3668 的 run-lifecycle 论证)。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FTszibd6C8sUCCZnM4VcrL --- .../ci-filter-implicit-success-guard.md | 24 ++++ .github/workflows/ci.yml | 109 ++++++++++++++++-- 2 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 .changeset/ci-filter-implicit-success-guard.md diff --git a/.changeset/ci-filter-implicit-success-guard.md b/.changeset/ci-filter-implicit-success-guard.md new file mode 100644 index 0000000000..7490d9160a --- /dev/null +++ b/.changeset/ci-filter-implicit-success-guard.md @@ -0,0 +1,24 @@ +--- +--- + +CI-only: a failed `filter` job no longer skips all seven core gates while branch +protection reports green. Releases nothing. + +`ci.yml`'s downstream jobs were guarded by `if: needs.filter.outputs.core == +'true'`, which names no status function — so GitHub wrapped it in an implicit +`success()`. Any `filter` failure (checkout flake, a `dorny/paths-filter` fault, +the 10-minute timeout) skipped `test` / `temporal-conformance` / `dogfood` / +`dogfood-verify` / `build-core` / `build-docs` / `console-pin` at once, and a +skipped required check counts as a pass: zero tests ran, nothing went red, the +PR was mergeable. All seven now read `if: ${{ !cancelled() && +needs.filter.outputs. != 'false' }}` — skip only when the filter +EXPLICITLY said false — which is the same "when in doubt, run everything" +trade-off the `|| 'true'` on the filter's own outputs already made. + +`test-gate` and `dogfood-gate` additionally take `filter` into `needs` and +refuse to accept a `skipped` leg when `filter` did not succeed, so the invariant +is asserted rather than merely arranged. `cancelled` keeps passing both gates +(#3668's run-lifecycle reasoning) and no other state changes behavior. + +Third sighting of this GitHub semantic; the other two were `release.yml`'s +publish-integrity guard and its `docker` job (#4900). diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 63080805ae..592878ef25 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,37 @@ jobs: docs: ${{ steps.changes.outputs.docs || 'true' }} core: ${{ steps.changes.outputs.core || 'true' }} console: ${{ steps.changes.outputs.console || 'true' }} + # ── THE FILTER CONTRACT, both halves (#4928) ────────────────────────── + # + # Half 1 is the `|| 'true'` above: when in doubt, RUN EVERYTHING. It + # covers a missing output VALUE. + # + # Half 2 is every downstream `if:` in this file, and it used to be + # missing. GitHub wraps an `if:` that names no status function in an + # IMPLICIT success(), so the original `if: needs.filter.outputs.core == + # 'true'` had TWO independent paths to "skip" when this job DIED + # (checkout flake, a dorny/paths-filter fault, the 10-minute timeout): + # the implicit success() is false, and the output is no longer a + # trustworthy 'true'. Both roads led to skipping test / dogfood / + # build-core — and skipped counts as SUCCESS in branch protection, so a + # single flake here produced a fully green, zero-test-run, mergeable PR + # with no red signal anywhere. + # + # So every downstream job now spells the contract as "skip only when the + # filter EXPLICITLY said false": + # + # if: ${{ !cancelled() && needs.filter.outputs. != 'false' }} + # + # `!cancelled()` displaces the implicit success() (a genuine run-level + # cancellation still skips — #3668's lifecycle reasoning); `!= 'false'` + # makes the empty string mean "run", exactly like `|| 'true'` does. This + # is deliberately robust to BOTH readings of what a failed job's outputs + # are — null/empty or the `|| 'true'` fallback — because `'' != 'false'` + # and `'true' != 'false'` are both true. The old form was broken under + # both, because the implicit success() dominates either way. + # + # Third occurrence of this GitHub semantic in one audit; the other two + # were release.yml's publish-integrity guard and its docker job (#4900). steps: - name: Checkout repository uses: actions/checkout@v7 @@ -94,7 +125,9 @@ jobs: # (the #3622 lesson; see dogfood-gate). name: Test Core (${{ matrix.shard }}/3) needs: filter - if: needs.filter.outputs.core == 'true' + # "Skip only when the filter EXPLICITLY said no core paths changed" — see + # THE FILTER CONTRACT on the filter job's outputs (#4928). + if: ${{ !cancelled() && needs.filter.outputs.core != 'false' }} runs-on: ubuntu-latest # Backstop only — the stall guard on the test steps is the primary # detector for a #4250-style hang and fires well before this. 30 min is @@ -277,7 +310,7 @@ jobs: # dogfood-gate for why `cancelled` passes and why this must not be # `if: !cancelled()` on the job. name: Test Core - needs: test + needs: [test, filter] if: always() runs-on: ubuntu-latest timeout-minutes: 10 @@ -287,7 +320,29 @@ jobs: - name: Verify test shard results run: | result="${{ needs.test.result }}" - echo "test matrix aggregate result: $result" + filter_result="${{ needs.filter.result }}" + echo "test matrix aggregate result: $result (filter job: $filter_result)" + # `skipped` passes this gate, but only when `filter` is the thing that + # decided it. `skipped` alone cannot tell "the path filter said no + # core paths changed" apart from "the path filter itself exploded and + # took every downstream job with it" — #4928, where the second case + # published a green Test Core over zero test runs. The `if:` on the + # `test` job above now makes a filter failure RUN the suite rather + # than skip it, so this branch should be unreachable; it is kept as + # the standing assertion of that invariant, because the failure mode + # it guards is silent and the `if:` is one careless edit from coming + # back. `filter` is in `needs` for exactly this read. + # + # `cancelled` on either side stays a pass, for #3668's reason spelled + # out in dogfood-gate below: cancellation is a run-lifecycle state + # (cancel-in-progress supersession), not a verdict, and failing here + # would paint a false red on the superseded SHA. + if [ "$result" = "skipped" ] \ + && [ "$filter_result" != "success" ] \ + && [ "$filter_result" != "cancelled" ]; then + echo "::error::Test Core shards were skipped while the filter job did not succeed (filter result: $filter_result). Refusing to report a pass over zero test runs — see #4928." + exit 1 + fi case "$result" in success|skipped|cancelled) echo "Test Core gate satisfied ($result)." ;; *) echo "::error::Test Core shards did not pass (aggregate result: $result)"; exit 1 ;; @@ -322,7 +377,8 @@ jobs: temporal-conformance: name: Temporal Conformance (live PG + MySQL) needs: filter - if: needs.filter.outputs.core == 'true' + # See THE FILTER CONTRACT on the filter job's outputs (#4928). + if: ${{ !cancelled() && needs.filter.outputs.core != 'false' }} runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -510,7 +566,8 @@ jobs: # dogfood-gate job below — the shard count can change without touching it. name: Dogfood Regression Gate (${{ matrix.shard }}/3) needs: filter - if: needs.filter.outputs.core == 'true' + # See THE FILTER CONTRACT on the filter job's outputs (#4928). + if: ${{ !cancelled() && needs.filter.outputs.core != 'false' }} runs-on: ubuntu-latest # Backstop only — the stall guard on the test step is the primary detector # for a #4250-style hang (see Test Core). 30 min is ~6× a shard (~5 min; @@ -639,7 +696,8 @@ jobs: dogfood-verify: name: Dogfood Verify CLI needs: filter - if: needs.filter.outputs.core == 'true' + # See THE FILTER CONTRACT on the filter job's outputs (#4928). + if: ${{ !cancelled() && needs.filter.outputs.core != 'false' }} runs-on: ubuntu-latest timeout-minutes: 20 permissions: @@ -727,9 +785,11 @@ jobs: # the split. # # `if: always()` + result inspection so a legitimately skipped matrix (the - # `filter` job says no core paths changed) still satisfies the gate. + # `filter` job says no core paths changed) still satisfies the gate — + # LEGITIMATELY being the operative word since #4928: `filter` must have + # concluded success (or the run been cancelled) for a skip to count. name: Dogfood Regression Gate - needs: [dogfood, dogfood-verify] + needs: [dogfood, dogfood-verify, filter] if: always() runs-on: ubuntu-latest timeout-minutes: 10 @@ -755,10 +815,27 @@ jobs: # #3622 merge-deadlock all over again. verify_result="${{ needs['dogfood-verify'].result }}" echo "dogfood-verify result: $verify_result" + # `skipped` is only a legitimate pass when the `filter` job is what + # decided it — the same #4928 hole test-gate documents above. A leg + # that is skipped while `filter` FAILED means the regression suite + # never ran and nothing anywhere is red. The `if:` conditions on + # dogfood / dogfood-verify make that unreachable today; this is the + # standing assertion that keeps it so. `cancelled` on `filter` still + # passes, same lifecycle reasoning as the paragraph above. + filter_result="${{ needs.filter.result }}" + echo "filter job result: $filter_result" fail=0 for r in "dogfood:$result" "dogfood-verify:$verify_result"; do case "${r#*:}" in - success|skipped|cancelled) echo "Gate leg ${r%%:*} satisfied (${r#*:})." ;; + skipped) + if [ "$filter_result" != "success" ] && [ "$filter_result" != "cancelled" ]; then + echo "::error::Gate leg ${r%%:*} was skipped while the filter job did not succeed (filter result: $filter_result). Refusing to report a pass over zero runs — see #4928." + fail=1 + else + echo "Gate leg ${r%%:*} satisfied (skipped by filter)." + fi + ;; + success|cancelled) echo "Gate leg ${r%%:*} satisfied (${r#*:})." ;; *) echo "::error::Gate leg ${r%%:*} did not pass (result: ${r#*:})"; fail=1 ;; esac done @@ -767,7 +844,11 @@ jobs: build-core: name: Build Core needs: filter - if: needs.filter.outputs.core == 'true' + # See THE FILTER CONTRACT on the filter job's outputs (#4928). No + # aggregation gate stands behind this job — its own name IS the required + # context — so this `if:` is the only thing between a filter flake and a + # green "Build Core" that built nothing. + if: ${{ !cancelled() && needs.filter.outputs.core != 'false' }} runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -914,7 +995,9 @@ jobs: build-docs: name: Build Docs needs: filter - if: needs.filter.outputs.docs == 'true' + # See THE FILTER CONTRACT on the filter job's outputs (#4928). `docs`, not + # `core` — each job keeps its own filter output. + if: ${{ !cancelled() && needs.filter.outputs.docs != 'false' }} runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -1000,7 +1083,9 @@ jobs: console-pin: name: Console Pin Gate needs: filter - if: needs.filter.outputs.console == 'true' + # See THE FILTER CONTRACT on the filter job's outputs (#4928). `console`, + # not `core` — each job keeps its own filter output. + if: ${{ !cancelled() && needs.filter.outputs.console != 'false' }} runs-on: ubuntu-latest timeout-minutes: 45 permissions: