Mark superseded commits honestly instead of skipped (closes #152)

Gitea cancels an in-flight run when a newer commit lands on the same
branch and records the cancellation as `failure` / "Has been
cancelled". The workflow rewrote that to `skipped`, but Gitea's
Combine() folds `skipped` into `success`, so the combined-status API
returned green for a commit nothing had ever tested. Rewrite it to
`failure` / "Superseded by a newer commit; never tested" instead:
red-but-honest, and never `pending`, which would block the commit
forever.

Re-running the superseded commit would have been better still, but is
not reachable on this Gitea (1.25.4): its API exposes no rerun
endpoint, workflow dispatch takes a ref rather than a SHA, and every
replay would be a full uncached build with no bound on how many pile
up behind a burst of merges.

The step also stops hardcoding its status context: the logic moves into
script/ci-mark-superseded, which derives the context from the workflow
name, job name and event -- the same three values Gitea builds it from
-- and fails loudly when no status on the commit being built carries
that context, so renaming the workflow or the job cannot silently
disable the rewrite. That is item 2 of
#147; item 1 there is
untouched.

Tests drive the script against a fake Gitea covering the cancelled,
laundered-skipped, genuinely-failed, passing and renamed cases, so jq
joins the builder image to run them.

.gitea/workflows/check.yml

@@ -16,36 +16,15 @@ jobs:
           # that touched the Docker build context.
           fetch-depth: 0
 
-      - name: Neutralize superseded run statuses
+      - name: Mark superseded run statuses
         # Gitea cancels the in-flight run when another commit is pushed to the
         # same branch and records the cancellation as `failure`, so a commit
-        # that was never tested reads red. The cancellation is unconditional
+        # that was never tested reads as a test result. The script rewrites
-        # server-side for push events and cannot be disabled from a workflow
+        # those statuses to say what happened. See its header for why the
-        # file, so the superseding run rewrites those statuses to `skipped`.
+        # state stays `failure` and not `skipped`.
-        # Only the exact cancellation status is touched; a real failure is
-        # left alone.
         env:
           GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
-        run: |
+        run: script/ci-mark-superseded
-          set -eu
-          api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
-          ctx='check / check (push)'
-          for sha in $(git rev-list --max-count=20 "${GITHUB_SHA}^" || true); do
-            latest="$(curl -sf "${api}/commits/${sha}/status" | jq -r \
-              --arg c "$ctx" \
-              '[.statuses[] | select(.context == $c)][0] // empty
-               | "\(.status)|\(.description)"')" || continue
-            [ "$latest" = 'failure|Has been cancelled' ] || continue
-            curl -sf -X POST "${api}/statuses/${sha}" \
-              -H "Authorization: token ${GITEA_TOKEN}" \
-              -H 'Content-Type: application/json' \
-              -d "$(jq -nc --arg c "$ctx" '{
-                context: $c,
-                state: "skipped",
-                description: "Superseded by a newer commit; never tested"
-              }')" >/dev/null
-            echo "neutralized superseded status on ${sha}"
-          done
 
       - name: Fingerprint the build context
         # `.dockerignore` keeps docs out of the build context, so a docs-only

Dockerfile

@@ -32,7 +32,9 @@ FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a349228
 # Depend on lint stage passing
 COPY --from=lint /src/go.sum /dev/null
 
-RUN apt-get update && apt-get install -y --no-install-recommends make && rm -rf /var/lib/apt/lists/*
+# jq is a runtime dependency of script/ci-mark-superseded, which the test
+# suite executes.
+RUN apt-get update && apt-get install -y --no-install-recommends make jq && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /build
 

README.md

@@ -255,6 +255,8 @@ them. We provide:
 - `script/docker` — build the Docker image tagged via `script/projectname`
 - `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
   runs the checks, so a green build implies a green repo)
+- `script/ci-mark-superseded` — CI helper: mark the commits whose run a
+  newer push cancelled (see [CI gate honesty](#ci-gate-honesty))
 - `script/precommit` — pre-commit checks (`go mod tidy` guard, then
   `script/check`)
 - `script/install-precommit` — install the git pre-commit hook that
@@ -1205,11 +1207,29 @@ way.
 
 The workflow's first step covers a second way the gate lied: Gitea
 cancels an in-flight run when a newer commit lands on the same branch
-and records that cancellation as a `failure` status, marking a commit
+and records that cancellation as a `failure` status, so a commit nothing
-red that was never tested. Cancellation is unconditional server-side for
+ever tested reads as a test result. Cancellation is unconditional
-push events, so the superseding run rewrites the exact
+server-side for push events, so the superseding run calls
-`Has been cancelled` status to `skipped`. Genuine failures are never
+`script/ci-mark-superseded`, which rewrites that exact status to
-touched.
+`failure` / `Superseded by a newer commit; never tested`.
+
+The state stays `failure` on purpose: Gitea's combined status folds
+`skipped` into `success`, so marking a never-tested commit `skipped`
+made the status API report green for it, indistinguishable from a commit
+that passed. Reading a commit's status on this repo therefore goes:
+
+- `success` / `Successful in ...` — the checks ran and passed.
+- `failure` / `Failing after ...` — the checks ran and failed.
+- `failure` / `Superseded by a newer commit; never tested` — the run was
+  cancelled by a newer push, and nothing was verified about this commit.
+  Test the commit itself before concluding anything about it.
+
+Genuine failures and successes are never touched, and no status is left
+`pending`, which would block the commit indefinitely. The step derives
+its context string from the workflow name, job name and event — the same
+three values Gitea builds the context from — and fails loudly when no
+status carries that context, so renaming the workflow or the job cannot
+silently disable the rewrite.
 
 ## TODO
 

go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/stretchr/testify v1.8.4
 	go.uber.org/fx v1.20.1
 	golang.org/x/crypto v0.38.0
+	gopkg.in/yaml.v3 v3.0.1
 	gorm.io/driver/sqlite v1.5.4
 	gorm.io/gorm v1.25.5
 	modernc.org/sqlite v1.28.0
@@ -52,7 +53,6 @@ require (
 	golang.org/x/text v0.25.0 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
 	lukechampine.com/uint128 v1.2.0 // indirect
 	modernc.org/cc/v3 v3.40.0 // indirect
 	modernc.org/ccgo/v3 v3.16.13 // indirect

internal/ciscript/ci_mark_superseded_test.go

@@ -0,0 +1,306 @@
+package ciscript_test
+
+import (
+	"maps"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
+)
+
+const (
+	// supersededDesc is the description script/ci-mark-superseded
+	// writes, and the one an earlier revision of it wrote alongside a
+	// `skipped` state.
+	supersededDesc = "Superseded by a newer commit; never tested"
+
+	// liveContext is the commit-status context Gitea uses for this
+	// repository's runs, as seen in its API. The script derives it from
+	// the workflow and job names rather than hardcoding it; the
+	// derivation is checked against this value below.
+	liveContext = "check / check (push)"
+
+	scriptPath = "../../script/ci-mark-superseded"
+	workflow   = "../../.gitea/workflows/check.yml"
+
+	// failure is the only state that neither folds into a combined
+	// `success` (as `skipped` does) nor blocks the commit forever (as
+	// `pending` does).
+	failure = "failure"
+)
+
+// repo is a throwaway git history: parent is the commit a run would be
+// cancelled on, head the commit that superseded it.
+type repo struct {
+	dir    string
+	head   string
+	parent string
+}
+
+// scriptEnv is the run identity the Gitea runner exports and the script
+// builds its context string from.
+type scriptEnv struct {
+	workflow string
+	job      string
+	event    string
+}
+
+func defaultEnv() scriptEnv {
+	return scriptEnv{workflow: "check", job: "check", event: "push"}
+}
+
+func cancelled() commitStatus {
+	return commitStatus{
+		Context:     liveContext,
+		Status:      failure,
+		Description: "Has been cancelled",
+	}
+}
+
+func running() commitStatus {
+	return commitStatus{
+		Context:     liveContext,
+		Status:      "pending",
+		Description: "Has started running",
+	}
+}
+
+func TestMarkSuperseded(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		parent   commitStatus
+		wantMark bool
+	}{
+		"a cancelled run is marked": {
+			parent:   cancelled(),
+			wantMark: true,
+		},
+		"a laundered skipped status is marked": {
+			parent: commitStatus{
+				Context:     liveContext,
+				Status:      "skipped",
+				Description: supersededDesc,
+			},
+			wantMark: true,
+		},
+		"a genuine failure is left alone": {
+			parent: commitStatus{
+				Context:     liveContext,
+				Status:      failure,
+				Description: "Failing after 3m1s",
+			},
+			wantMark: false,
+		},
+		"a passing run is left alone": {
+			parent: commitStatus{
+				Context:     liveContext,
+				Status:      "success",
+				Description: "Successful in 2m52s",
+			},
+			wantMark: false,
+		},
+		"another context is left alone": {
+			parent: commitStatus{
+				Context:     "other / other (push)",
+				Status:      failure,
+				Description: "Has been cancelled",
+			},
+			wantMark: false,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			requireTools(t)
+
+			history := newRepo(t)
+			fake, api := newFakeGitea(t)
+			fake.setStatus(history.head, running())
+			fake.setStatus(history.parent, tc.parent)
+
+			out, err := runScript(t, history, api, defaultEnv())
+			require.NoError(t, err, out)
+
+			posted := fake.postedFor(history.parent)
+			if !tc.wantMark {
+				require.Empty(t, posted)
+
+				return
+			}
+
+			require.Equal(t, []postedStatus{{
+				Context: liveContext,
+				// Not `skipped`: Gitea's combined status folds
+				// that into `success`, which is what made a
+				// never-tested commit read green.
+				State:       failure,
+				Description: supersededDesc,
+			}}, posted)
+		})
+	}
+}
+
+// A second run must not rewrite what the first one wrote, or every
+// later push would post a duplicate status.
+func TestMarkSupersededIsIdempotent(t *testing.T) {
+	t.Parallel()
+	requireTools(t)
+
+	history := newRepo(t)
+	fake, api := newFakeGitea(t)
+	fake.setStatus(history.head, running())
+	fake.setStatus(history.parent, cancelled())
+
+	for range 2 {
+		out, err := runScript(t, history, api, defaultEnv())
+		require.NoError(t, err, out)
+	}
+
+	require.Len(t, fake.postedFor(history.parent), 1)
+}
+
+// Renaming the workflow or the job changes the context string Gitea
+// uses. The script must say so instead of quietly matching nothing.
+func TestMarkSupersededRejectsAnUnknownContext(t *testing.T) {
+	t.Parallel()
+	requireTools(t)
+
+	history := newRepo(t)
+	fake, api := newFakeGitea(t)
+	fake.setStatus(history.head, running())
+	fake.setStatus(history.parent, cancelled())
+
+	env := defaultEnv()
+	env.job = "renamed"
+
+	out, err := runScript(t, history, api, env)
+	require.Error(t, err)
+	require.Contains(t, out, "renamed")
+	require.Contains(t, out, liveContext)
+	require.Empty(t, fake.postedFor(history.parent))
+}
+
+// The derived context must equal the one Gitea actually uses, which is
+// built from the same workflow and job names.
+func TestDerivedContextMatchesGitea(t *testing.T) {
+	t.Parallel()
+	requireTools(t)
+
+	name, job := workflowIdentity(t)
+
+	history := newRepo(t)
+	fake, api := newFakeGitea(t)
+	fake.setStatus(history.head, running())
+	fake.setStatus(history.parent, cancelled())
+
+	out, err := runScript(t, history, api, scriptEnv{
+		workflow: name,
+		job:      job,
+		event:    "push",
+	})
+	require.NoError(t, err, out)
+
+	posted := fake.postedFor(history.parent)
+	require.Len(t, posted, 1)
+	require.Equal(t, liveContext, posted[0].Context)
+}
+
+// workflowIdentity reads the workflow name and its single job id out of
+// the checked-in workflow file.
+func workflowIdentity(t *testing.T) (string, string) {
+	t.Helper()
+
+	raw, err := os.ReadFile(workflow)
+	require.NoError(t, err)
+
+	var parsed struct {
+		Name string         `yaml:"name"`
+		Jobs map[string]any `yaml:"jobs"`
+	}
+
+	require.NoError(t, yaml.Unmarshal(raw, &parsed))
+
+	jobs := slices.Collect(maps.Keys(parsed.Jobs))
+	require.Len(t, jobs, 1)
+
+	return parsed.Name, jobs[0]
+}
+
+func runScript(
+	t *testing.T, history repo, api string, env scriptEnv,
+) (string, error) {
+	t.Helper()
+
+	script, err := filepath.Abs(scriptPath)
+	require.NoError(t, err)
+
+	//nolint:gosec // fixed argv, repo-local script under test
+	cmd := exec.CommandContext(t.Context(), "sh", script)
+	cmd.Dir = history.dir
+	cmd.Env = append(os.Environ(),
+		"GITHUB_API_URL="+api,
+		"GITHUB_REPOSITORY=sneak/webhooker",
+		"GITHUB_SHA="+history.head,
+		"GITHUB_WORKFLOW="+env.workflow,
+		"GITHUB_JOB="+env.job,
+		"GITHUB_EVENT_NAME="+env.event,
+		"GITEA_TOKEN=test-token",
+	)
+
+	out, err := cmd.CombinedOutput()
+
+	return string(out), err
+}
+
+func newRepo(t *testing.T) repo {
+	t.Helper()
+
+	dir := t.TempDir()
+
+	git := func(args ...string) string {
+		//nolint:gosec // fixed argv, arguments are test constants
+		cmd := exec.CommandContext(t.Context(), "git", args...)
+		cmd.Dir = dir
+
+		out, err := cmd.CombinedOutput()
+		require.NoError(t, err, string(out))
+
+		return strings.TrimSpace(string(out))
+	}
+
+	commit := func(message string) string {
+		git(
+			"-c", "user.email=ci@example.invalid",
+			"-c", "user.name=ci",
+			"-c", "commit.gpgsign=false",
+			"commit", "-q", "--allow-empty", "-m", message,
+		)
+
+		return git("rev-parse", "HEAD")
+	}
+
+	git("init", "-q", "-b", "main")
+
+	parent := commit("parent")
+	head := commit("head")
+
+	return repo{dir: dir, head: head, parent: parent}
+}
+
+func requireTools(t *testing.T) {
+	t.Helper()
+
+	for _, tool := range []string{"sh", "git", "curl", "jq"} {
+		_, err := exec.LookPath(tool)
+		if err != nil {
+			t.Skipf("%s is not installed: %v", tool, err)
+		}
+	}
+}

internal/ciscript/doc.go

@@ -0,0 +1,5 @@
+// Package ciscript holds the tests for the repository's CI shell
+// scripts in script/. It carries no runtime code: the scripts run on
+// the CI runner, not inside the binary, but their behaviour still has
+// to be verified by the test suite.
+package ciscript

internal/ciscript/fakegitea_test.go

@@ -0,0 +1,142 @@
+package ciscript_test
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+)
+
+// commitStatus is the part of an entry in Gitea's combined-status
+// response that script/ci-mark-superseded reads.
+type commitStatus struct {
+	Context     string `json:"context"`
+	Status      string `json:"status"`
+	Description string `json:"description"`
+}
+
+// postedStatus is the part of a create-status request body the script
+// writes.
+type postedStatus struct {
+	Context     string `json:"context"`
+	State       string `json:"state"`
+	Description string `json:"description"`
+}
+
+// fakeGitea serves the two endpoints the script talks to. Like Gitea,
+// the newest status for a context replaces the previous one, so a
+// second run of the script sees what the first one wrote.
+type fakeGitea struct {
+	mu       sync.Mutex
+	statuses map[string][]commitStatus
+	posted   map[string][]postedStatus
+}
+
+// newFakeGitea returns the fake and the base URL to hand the script as
+// GITHUB_API_URL.
+func newFakeGitea(t *testing.T) (*fakeGitea, string) {
+	t.Helper()
+
+	fake := &fakeGitea{
+		mu:       sync.Mutex{},
+		statuses: map[string][]commitStatus{},
+		posted:   map[string][]postedStatus{},
+	}
+
+	srv := httptest.NewServer(fake.routes())
+	t.Cleanup(srv.Close)
+
+	return fake, srv.URL
+}
+
+func (f *fakeGitea) routes() http.Handler {
+	mux := http.NewServeMux()
+
+	mux.HandleFunc(
+		"GET /repos/{owner}/{repo}/commits/{sha}/status",
+		f.handleCombined,
+	)
+	mux.HandleFunc(
+		"POST /repos/{owner}/{repo}/statuses/{sha}",
+		f.handleCreate,
+	)
+
+	return mux
+}
+
+func (f *fakeGitea) handleCombined(
+	w http.ResponseWriter, r *http.Request,
+) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	body := struct {
+		Statuses []commitStatus `json:"statuses"`
+	}{Statuses: f.statuses[r.PathValue("sha")]}
+
+	payload, err := json.Marshal(body)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+
+		return
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+
+	_, _ = w.Write(payload)
+}
+
+func (f *fakeGitea) handleCreate(w http.ResponseWriter, r *http.Request) {
+	var got postedStatus
+
+	err := json.NewDecoder(r.Body).Decode(&got)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+
+		return
+	}
+
+	sha := r.PathValue("sha")
+
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	f.posted[sha] = append(f.posted[sha], got)
+	f.replaceLocked(sha, commitStatus{
+		Context:     got.Context,
+		Status:      got.State,
+		Description: got.Description,
+	})
+
+	w.WriteHeader(http.StatusCreated)
+}
+
+// setStatus gives a commit its latest status for a context.
+func (f *fakeGitea) setStatus(sha string, status commitStatus) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	f.replaceLocked(sha, status)
+}
+
+// postedFor returns the statuses the script created for a commit.
+func (f *fakeGitea) postedFor(sha string) []postedStatus {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	return append([]postedStatus(nil), f.posted[sha]...)
+}
+
+// replaceLocked requires f.mu.
+func (f *fakeGitea) replaceLocked(sha string, status commitStatus) {
+	for i, existing := range f.statuses[sha] {
+		if existing.Context == status.Context {
+			f.statuses[sha][i] = status
+
+			return
+		}
+	}
+
+	f.statuses[sha] = append(f.statuses[sha], status)
+}

script/ci-mark-superseded

@@ -0,0 +1,86 @@
+#!/bin/sh
+# script/ci-mark-superseded: record an honest status on commits whose CI
+# run Gitea cancelled because a newer commit landed on the same branch.
+# Gitea writes `failure` / "Has been cancelled" for such a run, which
+# reads as a test result on a commit nothing ever tested. Cancellation is
+# unconditional server-side for push events, so the superseding run
+# rewrites those statuses to `failure` with a description that says the
+# commit was never tested. `skipped` cannot be used: Gitea's combined
+# status folds `skipped` into `success`, so a never-tested commit would
+# report green. Genuine failures and successes are never touched.
+#
+# Called by the Gitea Actions workflow, which supplies GITHUB_API_URL,
+# GITHUB_REPOSITORY, GITHUB_SHA, GITHUB_WORKFLOW, GITHUB_JOB,
+# GITHUB_EVENT_NAME and GITEA_TOKEN. ANCESTOR_LIMIT (default 20) caps how
+# far back the walk looks.
+set -eu
+
+SUPERSEDED_DESC='Superseded by a newer commit; never tested'
+
+# Gitea builds the commit-status context as
+# "<workflow name> / <job name> (<event>)", the same three values the
+# runner exports, so derive it rather than hardcoding the result.
+context() {
+    printf '%s / %s (%s)' \
+        "$GITHUB_WORKFLOW" "$GITHUB_JOB" "$GITHUB_EVENT_NAME"
+}
+
+# The status Gitea created for this very job proves which context string
+# it uses. If the derived one is missing, the workflow or the job was
+# renamed and the match below would silently stop firing, restoring the
+# false-red bug with no signal. Fail loudly instead.
+require_own_context() {
+    if ! _body="$(curl -sf --retry 3 --retry-delay 2 --max-time 30 \
+        "${1}/commits/${GITHUB_SHA}/status")"; then
+        echo "cannot read commit statuses for ${GITHUB_SHA}" >&2
+        return 1
+    fi
+    _found="$(printf '%s' "$_body" | jq -r '(.statuses // [])[].context')"
+    if printf '%s\n' "$_found" | grep -qxF "$2"; then
+        return 0
+    fi
+    echo "no commit status with context '${2}' on ${GITHUB_SHA}:" >&2
+    echo "workflow or job renamed? contexts present:" >&2
+    printf '%s\n' "$_found" >&2
+    return 1
+}
+
+# Latest status for our context on a commit, as "state|description".
+status_of() {
+    curl -sf "${1}/commits/${2}/status" | jq -r --arg c "$3" \
+        '[(.statuses // [])[] | select(.context == $c)][0] // empty
+         | "\(.status)|\(.description)"'
+}
+
+mark_superseded() {
+    curl -sf -X POST "${1}/statuses/${2}" \
+        -H "Authorization: token ${GITEA_TOKEN}" \
+        -H 'Content-Type: application/json' \
+        -d "$(jq -nc --arg c "$3" --arg d "$SUPERSEDED_DESC" \
+            '{context: $c, state: "failure", description: $d}')" \
+        >/dev/null
+}
+
+main() {
+    _api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
+    _ctx="$(context)"
+
+    require_own_context "$_api" "$_ctx"
+
+    _walk="$(git rev-list \
+        --max-count="${ANCESTOR_LIMIT:-20}" "${GITHUB_SHA}^" || true)"
+
+    for _sha in $_walk; do
+        _latest="$(status_of "$_api" "$_sha" "$_ctx")"
+        # A run that was cancelled, or one an earlier revision of this
+        # script laundered into `skipped`. Anything else stands.
+        case "$_latest" in
+        'failure|Has been cancelled' | "skipped|${SUPERSEDED_DESC}") ;;
+        *) continue ;;
+        esac
+        mark_superseded "$_api" "$_sha" "$_ctx"
+        echo "marked superseded: ${_sha}"
+    done
+}
+
+main "$@"