CI-9: drop the pull_request branches filter, because a skipped check clears the gate
checks / unit (pull_request) Successful in 7m57s
checks / unit (pull_request) Successful in 7m57s
`branches: [release/1.0, master]` meant a stacked pull request - one based on another feature branch - never matched. Gitea does not stay silent about a workflow it sits out: it posts the required context itself with state `skipped`, the combined-status rollup reports that as `success`, and the merge gate accepts it. Retargeting does not repair it, because the skip is a commit status on the head sha and so survives the base change. Measured on Gitea 1.27.3 with throwaway PR #94 against a branch carrying a copy of release/1.0's protection rule: only status is `skipped` -> POST .../merge 409 head out of date (CLEARED) no status at all -> POST .../merge 405 not all required (refused) status `pending` -> POST .../merge 405 not all required (refused) status `success` -> POST .../merge 409 head out of date (CLEARED) The 409s come from a deliberately wrong head_commit_id in the probe, which Gitea checks only after branch protection - so a 409 means the request had already cleared the gate, and nothing was ever merged. `skipped` is the only state that leaks. Fixing this at the gate is not possible. Gitea branch protection can require that a context is PRESENT, never that it is SUCCESS: its whole status-check vocabulary is `enable_status_check` and `status_check_contexts`, per this server's own /swagger.v1.json. So the fix has to be upstream - never emit a skip - which means no event filter. Cost: every pull request now runs the full ~8 minute suite, including the intermediate steps of a stack. 49 of the 50 pull requests in this repository's history were opened straight against release/1.0 and would have run anyway. Also corrects two things CI-01's header stated as fact. A filter miss is fail-OPEN, not the fail-closed pending-forever it described; and it is a skip-ci keyword, not a filter miss, that actually withholds the context and leaves it pending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
@@ -3,12 +3,35 @@
|
|||||||
# of real coverage including a wall-clock cadence test, and not one machine ever
|
# of real coverage including a wall-clock cadence test, and not one machine ever
|
||||||
# ran them. Every merge here was a human reading a diff.
|
# ran them. Every merge here was a human reading a diff.
|
||||||
#
|
#
|
||||||
# ⚠️ NO `paths:` FILTER, AND ITS ABSENCE IS LOAD-BEARING. Branch protection on
|
# ⚠️ NO EVENT FILTER OF ANY KIND - NO `paths:`, AND SINCE CI-9 NO `branches:`
|
||||||
# release/1.0 requires the context `checks / unit (pull_request)`. Gitea leaves a
|
# EITHER - AND THAT ABSENCE IS LOAD-BEARING. Branch protection on release/1.0
|
||||||
# required context it never receives PENDING forever, so the moment this workflow
|
# requires the context `checks / unit (pull_request)`. Condition a STEP if you
|
||||||
# learns to sit a PR out, that PR becomes unmergeable. Condition a STEP if you
|
|
||||||
# must; never the workflow.
|
# must; never the workflow.
|
||||||
#
|
#
|
||||||
|
# ⚠️ CI-01 WROTE THE REASON DOWN WRONG, AND THE WRONG REASON WAS THE SAFE-
|
||||||
|
# SOUNDING ONE. It said an unreceived required context sits PENDING forever, so
|
||||||
|
# a sat-out pull request becomes unmergeable - annoying, but fail-safe. That is
|
||||||
|
# not what this Gitea does. MEASURED on Gitea 1.27.3, 2026-09-19, throwaway
|
||||||
|
# PR #94: when a workflow exists but its event filter does not match, Gitea does
|
||||||
|
# not stay silent. It POSTS the required context itself, with state `skipped`:
|
||||||
|
#
|
||||||
|
# GET /api/v1/repos/admin/goodwe-addon/commits/<head sha>/statuses
|
||||||
|
# [{"status":"skipped","context":"checks / unit (pull_request)",
|
||||||
|
# "description":"Skipped","target_url":""}] <- no run, no job, no url
|
||||||
|
#
|
||||||
|
# GET /api/v1/repos/admin/goodwe-addon/commits/<head sha>/status
|
||||||
|
# {"state":"success","total_count":1} <- the rollup LAUNDERS it
|
||||||
|
#
|
||||||
|
# and Gitea's merge gate accepts it. Same PR, same head sha, one status on it
|
||||||
|
# and that status a skip, POST .../pulls/94/merge answered 409 `head out of
|
||||||
|
# date` - the 409 being a deliberately wrong `head_commit_id` in the probe, i.e.
|
||||||
|
# the request had ALREADY CLEARED BRANCH PROTECTION. The control, a required
|
||||||
|
# context with no status at all, answered 405 `Not all required status checks
|
||||||
|
# successful`. So the true failure mode is the exact inverse of the one written
|
||||||
|
# here before: sitting a pull request out does not make it unmergeable, it makes
|
||||||
|
# it MERGEABLE WITHOUT EVER BEING TESTED, and the badge says success while it
|
||||||
|
# happens. Fail-open, not fail-closed. Do not reintroduce a filter here.
|
||||||
|
#
|
||||||
# ⚠️ THE RUNNER IS SHARED WITH THE OTHER REPO'S RIG. `goodwe-runner` on
|
# ⚠️ THE RUNNER IS SHARED WITH THE OTHER REPO'S RIG. `goodwe-runner` on
|
||||||
# 192.168.2.147 carries the labels `self-hosted` and `goodwe-rig` and is
|
# 192.168.2.147 carries the labels `self-hosted` and `goodwe-rig` and is
|
||||||
# registered `:host`, and it is the only runner there is - so this job wears the
|
# registered `:host`, and it is the only runner there is - so this job wears the
|
||||||
@@ -23,9 +46,69 @@
|
|||||||
# that step; a green from this workflow is a claim about content that can land.
|
# that step; a green from this workflow is a claim about content that can land.
|
||||||
name: checks
|
name: checks
|
||||||
|
|
||||||
|
# ⚠️ CI-9: `pull_request:` TAKES NO `branches:` LIST, ON PURPOSE. It used to
|
||||||
|
# read `branches: [release/1.0, master]`, which meant a STACKED pull request -
|
||||||
|
# one whose base is another feature branch rather than a release branch - did
|
||||||
|
# not match, and so was skipped, laundered to `success`, and mergeable untested
|
||||||
|
# by the mechanism described at the top of this file.
|
||||||
|
#
|
||||||
|
# ⚠️ AND RETARGETING DID NOT REPAIR IT, WHICH IS WHAT MADE THIS P1. The skip is
|
||||||
|
# a COMMIT STATUS ON THE HEAD SHA, not a property of the pull request, so moving
|
||||||
|
# the base afterwards leaves it exactly where it was. Observed live on PR #93
|
||||||
|
# (2026-09-19): retargeted TEL-10 -> release/1.0, and the 10:05:00 `skipped`
|
||||||
|
# stayed attached to head a6af332 with the rollup still reading success. It was
|
||||||
|
# caught by a human who forced a real execution by reopening the pull request
|
||||||
|
# (run 799, 10:56). Reproduced deliberately on throwaway PR #94: retargeted onto
|
||||||
|
# a branch carrying a byte-identical copy of release/1.0's protection rule, and
|
||||||
|
# the single `skipped` status survived the retarget unchanged - no new run, no
|
||||||
|
# new status - and still cleared the gate.
|
||||||
|
#
|
||||||
|
# ⚠️ WHY NOT FIX THIS AT THE GATE INSTEAD, which would be the better place. It
|
||||||
|
# cannot be done. Gitea's branch protection cannot express "this context must be
|
||||||
|
# SUCCESS" as opposed to "this context must be PRESENT AND NOT FAILING". The
|
||||||
|
# whole of its status-check vocabulary is two fields - `enable_status_check`
|
||||||
|
# (bool) and `status_check_contexts` (a list of globs MATCHING CONTEXT NAMES) -
|
||||||
|
# confirmed against this server's own schema at /swagger.v1.json, definitions
|
||||||
|
# EditBranchProtectionOption and CreateBranchProtectionOption. There is no field
|
||||||
|
# for the required STATE anywhere in either. The success/skip comparison lives
|
||||||
|
# in Gitea's compiled code, so the only lever left is upstream of it: make sure
|
||||||
|
# the context is never a skip in the first place. Hence this block.
|
||||||
|
#
|
||||||
|
# ⚠️ WHAT THIS COSTS, stated honestly, because it is a real trade. Every pull
|
||||||
|
# request now runs the full suite, including the intermediate steps of a stack
|
||||||
|
# that nobody intends to merge on its own. A run is ~8 minutes wall clock
|
||||||
|
# (run 799: 7m55s) on a runner that is `capacity: 1` and shared with the other
|
||||||
|
# repo's rig, so each extra run is ~8 minutes of slot that a rig campaign then
|
||||||
|
# queues behind. Measured scale of the new work: of the 50 pull requests in this
|
||||||
|
# repository's history, 49 were opened directly against release/1.0 and would
|
||||||
|
# have run anyway; stacking is rare here but not hypothetical - PR #93 was
|
||||||
|
# stacked on TEL-10 five days ago and is the reason this file is being edited.
|
||||||
|
# The bill is therefore small today and grows with however much stacking the
|
||||||
|
# team adopts. It is worth paying: the alternative is that the cheap case stays
|
||||||
|
# untested and indistinguishable from the tested one.
|
||||||
|
#
|
||||||
|
# ⚠️ THIS DOES NOT CLOSE THE `skipped` HOLE, IT REMOVES THIS REPOSITORY'S ONLY
|
||||||
|
# CAUSE OF ONE. Gitea's half - that a `skipped` required context is accepted by
|
||||||
|
# the merge gate - is untouched and uncloseable from here. What CI-9 can say is
|
||||||
|
# that after this edit there is no longer anything in this repository that
|
||||||
|
# produces one: there is exactly one workflow file, it now has no event filter
|
||||||
|
# of any kind, and the other candidate cause was MEASURED and is not one.
|
||||||
|
#
|
||||||
|
# THE MEASUREMENT, because the obvious guess is wrong and was nearly written
|
||||||
|
# here as fact. A skip-ci keyword in a commit message (`[skip ci]`, `[ci skip]`,
|
||||||
|
# `[no ci]`, `[skip actions]`, `[actions skip]`) does NOT behave like a filter
|
||||||
|
# miss. On throwaway PR #94, head b36effb, commit subject ending `[skip ci]`:
|
||||||
|
#
|
||||||
|
# .../commits/b36effb/statuses -> [] <- NO status, not a skip
|
||||||
|
# .../commits/b36effb/status -> {"state":"pending"} <- so the rollup is pending
|
||||||
|
# POST .../pulls/94/merge -> 405 Not all required status checks successful
|
||||||
|
#
|
||||||
|
# i.e. skip-ci is FAIL-CLOSED - it withholds the context entirely, which is the
|
||||||
|
# pending-forever behaviour CI-01's header wrongly attributed to a filter miss.
|
||||||
|
# It blocks the merge rather than laundering it, so it needs no fix and no
|
||||||
|
# app.ini change. Only the filter miss ever posted a `skipped`.
|
||||||
on:
|
on:
|
||||||
pull_request:
|
pull_request:
|
||||||
branches: [release/1.0, master]
|
|
||||||
workflow_dispatch:
|
workflow_dispatch:
|
||||||
|
|
||||||
# ⚠️ NO `concurrency:` BLOCK, AND ITS ABSENCE IS AS LOAD-BEARING AS THE MISSING
|
# ⚠️ NO `concurrency:` BLOCK, AND ITS ABSENCE IS AS LOAD-BEARING AS THE MISSING
|
||||||
|
|||||||
Reference in New Issue
Block a user