CI-02: grade the merge, and stop a cancelled run reading as a test failure
checks / unit (pull_request) Successful in 3m44s
checks / unit (pull_request) Successful in 3m44s
Two things a green from `checks / unit (pull_request)` was claiming without having observed. Both were measured on a deliberately conflicted throwaway pull request (#29, now closed and its branch deleted) rather than read off the workflow, because the Actions log endpoints 404 on this Gitea and a step's own conclusion is the only channel out of a job. CASE 1, AND THE TICKET'S MECHANISM IS WRONG IN A WAY THAT MAKES IT WORSE. The card says a conflicted pull request loses its merge ref, so that git fetch -q --depth 1 origin "$GITHUB_REF" \ || git fetch -q --depth 1 origin "$GITHUB_SHA" falls through to the second half and quietly grades the bare branch head. The symptom is real - PR #14 reported success while `mergeable: false` - but that is not how. Asking one question per step on the conflicted probe (runs 558, 559): GITHUB_REF == refs/pull/<n>/head P01 passed GITHUB_REF != refs/pull/<n>/merge P02 failed GITHUB_SHA == the pull request head sha P03 passed fetching GITHUB_REF succeeds P04 passed <- even conflicted ...and it lands on the bare head P05 passed refs/pull/<n>/merge does not exist P06 failed `git ls-remote origin 'refs/pull/*'` on this server advertises only `/head`, for open and closed pull requests alike. There are no merge refs here at all. The first fetch has therefore always succeeded and the fallback has never fired once; both halves name the same commit anyway. So the gate did not lose merged content on conflicted pull requests - it has never had merged content, on any pull request in this repository's history. Green has meant "this branch passed on its own", and with `block_on_outdated_branch` off on release/1.0 the base has usually moved underneath it. Reproduced end to end on one pull request, same conflict, gate swapped: run 567 the ORIGINAL gate -> success, "Successful in 3m38s" run 566 the gate below -> failure in 2s, merge step red, both test steps `skipped` The checkout now fetches the base tip and the pull request head, merges them, and refuses if the merge does not apply - as its own NAMED step, because /actions/runs/<id>/jobs carries a per-step conclusion and that is what lets a reader tell "nothing was tested" from "the tests spoke". `--depth 1` is gone: a merge needs a merge base, and a full fetch of this repository measured 6.8 MB and 1.1 s. The fallback survives only on the workflow_dispatch path, which has no pull request to merge and now announces which bare ref it took. CASE 2, AND `cancel-in-progress` IS NOT THE KNOB. Runs 531 and 533 on PR #14 were cancelled at 0s, each at the exact second the next run on that pull request was created: 531 created 17:08:19 `Waiting to run`, cancelled 17:10:44 = 532's creation 533 created 17:45:11 `Waiting to run`, cancelled 18:11:22 = 534's creation Both have `started_at` at the Unix epoch. A cancelled run posts `failure` on the required context, described `Canceled after 0s`; branch protection compares state, not description, so "nothing ran" and "your tests are broken" arrive as the same red wanting opposite responses. Three settings were measured, each with one run holding the single runner slot and two further pushes behind it: no concurrency block at all runs 561 and 562 both queued, both executed group, cancel-in-progress:false run 564 sat `Blocked by required conditions` and was cancelled at 0s the second 565 existed A shared group cancels a run that has not started regardless of that flag - GitHub documents it and Gitea 1.27 implements it. Having no group is the only configuration measured to leave a queued run alive, so the block is gone and the measurement is written above it. The cost is stated in the file: a developer who pushes four times occupies the shared runner four times. That is CI-01's own trade written the other way round - a delayed verdict, never a destroyed one - and a destroyed verdict is worse here because it is indistinguishable from a real failure. A sha-keyed group was considered and rejected: the only cancellations it still permits are same-sha ones, whose red lands on a commit that IS still the head. WHAT IS NOT ESTABLISHED, SAID PLAINLY. Nothing here makes a cancelled run distinguishable at the gate BY STATE; that is Gitea posting the status, not this workflow, and no edit to this file can change it. What ships removes the cause instead of relabelling the symptom. And a commit that never had a run carries an empty `/statuses/<sha>` list, while a queued one carries `pending / Waiting to run` within seconds and a concurrency-held one `pending / Blocked by required conditions` - those were already distinguishable at the API. `/actions/tasks`, which is what was polled when this was first reported, lists only tasks that started and cannot answer the question at all. test_ci_gate.py grades the gate by RUNNING it. It extracts the checkout and merge steps out of checks.yml and executes the shipped commands against two throwaway local repositories it builds - one whose pull request merges, one whose does not - and pins that the old one-liner exits 0 on the unmergeable one while the new steps refuse and say nothing was tested. Part 1 pins the two tokens that assemble `checks / unit (pull_request)`: renaming `name:` or the `unit:` job key would leave every open and future pull request waiting forever on a context that never arrives, with no error anywhere in that sequence, and admin override blocked. Seven mutations of checks.yml were each confirmed red, including both renames and a restored `--depth 1`. .gitea/ci/Dockerfile gains `git` for it; the fixtures are local paths and no network is touched. 17/17 green in the checks image. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+166
-13
@@ -18,7 +18,9 @@
|
||||
# so it QUEUES behind a rig campaign - a delayed verdict, never a destroyed one.
|
||||
#
|
||||
# NO JS ACTIONS: the runner image has no node, so `actions/checkout` cannot run.
|
||||
# Checkout is seven lines of git below.
|
||||
# Checkout is plain git below - and since CI-02 it is a checkout of the MERGE of
|
||||
# the pull request into its base, not of the branch tip. See the long note on
|
||||
# that step; a green from this workflow is a claim about content that can land.
|
||||
name: checks
|
||||
|
||||
on:
|
||||
@@ -26,15 +28,53 @@ on:
|
||||
branches: [release/1.0, master]
|
||||
workflow_dispatch:
|
||||
|
||||
concurrency:
|
||||
# ⚠️ PER PULL REQUEST, NOT A BARE NAME. A shared group holds one running and
|
||||
# one pending run, so a bare name lets a second PR supersede a first PR's run
|
||||
# before any runner has picked it up - which is exactly what happened to the
|
||||
# other repo's run 521 (four jobs, `started_at` at the Unix epoch,
|
||||
# `runner_id: 0`, concluded `cancelled`, and the PR merged anyway). Do not
|
||||
# replace this with a constant.
|
||||
group: addon-checks-${{ github.event.pull_request.number || github.ref }}
|
||||
cancel-in-progress: true
|
||||
# ⚠️ NO `concurrency:` BLOCK, AND ITS ABSENCE IS AS LOAD-BEARING AS THE MISSING
|
||||
# `paths:` FILTER ABOVE. CI-01 put one here - `group: addon-checks-<pr number>`,
|
||||
# `cancel-in-progress: true` - to stop a second pull request superseding a
|
||||
# first one's run. It did stop that. It also did this, twice, on PR #14:
|
||||
#
|
||||
# run 531 created 17:08:19 `pending / Waiting to run`
|
||||
# cancelled 17:10:44 <- the exact second run 532 was created
|
||||
# run 533 created 17:45:11 `pending / Waiting to run`
|
||||
# cancelled 18:11:22 <- the exact second run 534 was created
|
||||
#
|
||||
# Both cancelled runs have `started_at` at the Unix epoch: they never executed.
|
||||
# A cancelled run posts `failure` on the required context, described
|
||||
# `Canceled after 0s`. Branch protection compares STATE, not description, so
|
||||
# "nothing ran" and "your tests are broken" arrive at the gate as the same red,
|
||||
# and the correct operator response to each is the opposite of the other.
|
||||
#
|
||||
# CI-02 MEASURED THE THREE CANDIDATE SETTINGS ON A THROWAWAY PR (#29) RATHER
|
||||
# THAN READING THE DOCS, because the docs and this Gitea disagree about which
|
||||
# knob does it. In each trial one run was made to hold the single slot and two
|
||||
# further pushes followed:
|
||||
#
|
||||
# trial 1, NO concurrency block - runs 561 and 562 both sat queued together
|
||||
# and both executed. Nothing was cancelled.
|
||||
# trial 2, group kept, cancel-in-progress: FALSE
|
||||
# - run 564 sat `pending / Blocked by required
|
||||
# conditions` and was cancelled at 0s the
|
||||
# second run 565 was created.
|
||||
#
|
||||
# So `cancel-in-progress` IS NOT THE KNOB. A shared group cancels a run that has
|
||||
# not started no matter what that flag says - GitHub documents this ("any
|
||||
# previously pending run in the group will be cancelled") and Gitea 1.27
|
||||
# implements it. The only configuration that leaves a queued run alive is having
|
||||
# no group at all, which is what is written here.
|
||||
#
|
||||
# ⚠️ DO NOT REPLACE THIS WITH A SHA-KEYED GROUP as a compromise. It looks
|
||||
# tempting - two runs on different commits could never share a group, so nothing
|
||||
# would be cancelled - but the only cancellations it still permits are the
|
||||
# same-sha ones, and those are precisely the dangerous half: a red for a run
|
||||
# that never started, landing on a commit that IS still the head. It
|
||||
# concentrates the failure instead of removing it.
|
||||
#
|
||||
# WHAT THIS COSTS, stated honestly: the runner is `capacity: 1` and shared with
|
||||
# the other repo's rig, so a developer who pushes four times in a minute now
|
||||
# occupies it four times instead of once. That is CI-01's own trade written the
|
||||
# other way round - it accepted "a delayed verdict, never a destroyed one" for
|
||||
# the rig, and this accepts the same for pushes. A destroyed verdict here is
|
||||
# worse than a slow one, because it is indistinguishable from a real failure.
|
||||
|
||||
jobs:
|
||||
unit:
|
||||
@@ -58,19 +98,132 @@ jobs:
|
||||
# exist, so the test dies on `IndexError: 1` before its first assertion.
|
||||
# Measured during CI-01; the flat layout was tried first and did exactly
|
||||
# that. $WS/goodwe-addon/goodwe_controller gives parents[1] == $WS.
|
||||
- name: checkout into a path the host shares (git; no node for actions/checkout)
|
||||
#
|
||||
# ⚠️ WHAT THIS CHECKS OUT IS THE MERGE, NOT THE BRANCH, AND THAT IS THE
|
||||
# WHOLE OF CI-02. What was here before was one line:
|
||||
#
|
||||
# git fetch -q --depth 1 origin "$GITHUB_REF" \
|
||||
# || git fetch -q --depth 1 origin "$GITHUB_SHA"
|
||||
#
|
||||
# CI-02 measured what those two names actually are on this Gitea, on a
|
||||
# deliberately conflicted throwaway PR (#29, runs 558 and 559), by asking
|
||||
# one question per STEP - the Actions log endpoints 404 here, so a step's
|
||||
# own conclusion is the only channel out of a job. The answers:
|
||||
#
|
||||
# GITHUB_REF == refs/pull/<n>/head (step P01 passed)
|
||||
# GITHUB_REF != refs/pull/<n>/merge (step P02 failed)
|
||||
# GITHUB_SHA == the pull request head sha (step P03 passed)
|
||||
# fetching GITHUB_REF succeeds (step P04 passed)
|
||||
# ...and lands on the bare head (step P05 passed)
|
||||
# refs/pull/<n>/merge does not exist (step P06 failed)
|
||||
#
|
||||
# ⚠️ SO THE FALLBACK NEVER FIRED, AND THE DEFECT IS WORSE THAN THE ONE
|
||||
# REPORTED. The ticket says a conflicted PR loses its merge ref and the
|
||||
# `||` quietly grades the bare head instead. It does not: this Gitea has
|
||||
# no merge refs at all - `git ls-remote origin 'refs/pull/*'` advertises
|
||||
# only `/head`, for open and closed pull requests alike - so the FIRST
|
||||
# half always succeeds, and both halves name the same commit anyway
|
||||
# (GITHUB_REF resolves to GITHUB_SHA). The gate has never once graded
|
||||
# merged content, on any pull request in this repository's history. Green
|
||||
# has meant "this branch passed on its own", which on a base that has
|
||||
# moved - and `block_on_outdated_branch` is off here, so it usually has -
|
||||
# is not the content that lands. PR #14 reporting success while
|
||||
# `mergeable: false` was not the fallback misfiring; it was the gate
|
||||
# working exactly as written.
|
||||
#
|
||||
# A fallback that cannot fire cannot be "kept for the case it was for", so
|
||||
# it is gone from this path and kept, ANNOUNCED, on the one path that has
|
||||
# no pull request to merge (workflow_dispatch, below).
|
||||
#
|
||||
# ⚠️ NOT `--depth 1` ANY MORE. A merge needs the merge base, and a shallow
|
||||
# pair does not have one. Measured before changing it: a full fetch of
|
||||
# this repository is 6.8 MB and 1.1 s against this server, over 95
|
||||
# commits. The depth flag was buying nothing worth a wrong verdict.
|
||||
#
|
||||
# ⚠️ THE WORKFLOW AND THE CONTENT NOW COME FROM DIFFERENT TREES. Gitea
|
||||
# picks the workflow file from refs/pull/<n>/head, so THIS file is always
|
||||
# the branch's; the tree graded below - including .gitea/ci/Dockerfile -
|
||||
# is the merge. That is the right way round (a pull request must not be
|
||||
# able to grade itself with a gate the base has not seen) but it means a
|
||||
# change to this file is tested by the branch it is on, which is how you
|
||||
# are reading this.
|
||||
- name: check out the base branch and the pull request head (git; no node for actions/checkout)
|
||||
env:
|
||||
URL: ${{ github.server_url }}/${{ github.repository }}.git
|
||||
TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
PRNUM: ${{ github.event.pull_request.number }}
|
||||
BASEREF: ${{ github.base_ref }}
|
||||
run: |
|
||||
set -eu
|
||||
rm -rf "$WS" && mkdir -p "$WS/goodwe-addon" && cd "$WS/goodwe-addon"
|
||||
git init -q .
|
||||
git remote add origin "$(echo "$URL" | sed "s#://#://x-access-token:${TOKEN}@#")"
|
||||
git fetch -q --depth 1 origin "$GITHUB_REF" || git fetch -q --depth 1 origin "$GITHUB_SHA"
|
||||
git checkout -qf FETCH_HEAD
|
||||
if [ -n "$PRNUM" ]; then
|
||||
# Two fetches, not one, so that a failure to reach either ref fails
|
||||
# HERE with the ref in the message, rather than inside the merge
|
||||
# step where it would read as a conflict. `github.base_ref` being
|
||||
# populated at all was checked on the probe PR too (step Q01).
|
||||
git fetch -q origin "+refs/heads/$BASEREF:refs/remotes/origin/base"
|
||||
git fetch -q origin "+refs/pull/$PRNUM/head:refs/remotes/origin/pr"
|
||||
git checkout -qf refs/remotes/origin/base
|
||||
echo "base $BASEREF $(git rev-parse refs/remotes/origin/base)"
|
||||
echo "head refs/pull/$PRNUM/head $(git rev-parse refs/remotes/origin/pr)"
|
||||
else
|
||||
# NO PULL REQUEST (workflow_dispatch). There is nothing to merge
|
||||
# into, so this path grades a bare ref and SAYS SO - which is all
|
||||
# the old `||` ever needed to do. It is kept here because a branch
|
||||
# can be deleted or renamed between the dispatch and the fetch, and
|
||||
# unlike the pull_request path that is a case nobody has ruled out.
|
||||
if git fetch -q origin "$GITHUB_REF"; then
|
||||
echo "GRADED: the bare ref $GITHUB_REF - NOT merged content (no pull request in this event)."
|
||||
else
|
||||
git fetch -q origin "$GITHUB_SHA"
|
||||
echo "GRADED: the bare commit $GITHUB_SHA - NOT merged content, and NOT $GITHUB_REF either, which would not fetch."
|
||||
fi
|
||||
git checkout -qf FETCH_HEAD
|
||||
fi
|
||||
echo "checked out $(git log --oneline -1)"
|
||||
|
||||
# ⚠️ THIS STEP IS THE ONE THAT MAKES A GREEN MEAN SOMETHING. It exists as
|
||||
# its OWN step rather than as three lines of the checkout because the
|
||||
# Actions log endpoints 404 on this Gitea and
|
||||
# /api/v1/repos/.../actions/runs/<id>/jobs - which carries a per-step
|
||||
# conclusion - is the only machine-readable account of what a run did. A
|
||||
# reader who sees this step red knows the pull request does not apply and
|
||||
# that NO TEST RAN; a reader who sees the next two red knows the tests ran
|
||||
# and spoke. Collapse them back together and that distinction is gone
|
||||
# again, which is the shape of defect this whole ticket is about.
|
||||
#
|
||||
# The refusal is a FAILURE rather than a skip on purpose. "Reports
|
||||
# nothing" would leave the required context pending forever, which this
|
||||
# file's own header explains is unmergeable-forever; and a conflicted pull
|
||||
# request cannot be merged by Gitea anyway, so the red blocks nothing that
|
||||
# was not already blocked. What it does is stop a green being posted about
|
||||
# content that cannot land.
|
||||
- name: grade the merge, not the branch - refuse a pull request that does not apply
|
||||
env:
|
||||
PRNUM: ${{ github.event.pull_request.number }}
|
||||
BASEREF: ${{ github.base_ref }}
|
||||
run: |
|
||||
set -eu
|
||||
cd "$WS/goodwe-addon"
|
||||
if [ -z "$PRNUM" ]; then
|
||||
echo "no pull request in this event - nothing to merge, and the previous step said what it graded."
|
||||
exit 0
|
||||
fi
|
||||
if git -c user.email=checks@localhost -c user.name=checks \
|
||||
merge --no-edit refs/remotes/origin/pr; then
|
||||
echo "GRADED: refs/pull/$PRNUM/head merged into $BASEREF"
|
||||
echo " result $(git rev-parse HEAD)"
|
||||
echo " tree $(git rev-parse 'HEAD^{tree}')"
|
||||
else
|
||||
git merge --abort 2>/dev/null || true
|
||||
echo "REFUSED: refs/pull/$PRNUM/head does not merge into $BASEREF."
|
||||
echo " NOTHING HAS BEEN TESTED. This is not a test failure - no test ran."
|
||||
echo " Rebase or merge $BASEREF into the branch and push again."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: build the test image (shipped deps, from apk - see .gitea/ci/Dockerfile)
|
||||
run: |
|
||||
set -eu
|
||||
|
||||
Reference in New Issue
Block a user