From 499094b5bb72db160302be957b90dd3d40f723b0 Mon Sep 17 00:00:00 2001 From: Blair Hamilton Date: Tue, 4 Aug 2026 21:55:25 -0400 Subject: [PATCH] fix(pre-commit-advisory): stop a failed cache-seed run from reddening main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The push-to-default-branch run gates nothing. Its hook steps already end in `|| true`, and the step that reflects pre-commit's exit code as the job status is `pull_request`-only — the run exists solely to populate the default-branch cache scope so PR branches can restore hook envs. It could still fail on an *infrastructure* step, and did. A transient curl: (35) Recv failure: Connection reset by peer while downloading the go-pre-commit release archive failed the install step and turned an otherwise-green k5s main red (run 30964880559) — test and build both passed, nothing was wrong with the repo or its hooks, and a plain re-run went green with no change. Because callers pin `@main`, that failure mode was reachable from every service repo's default branch. A seed run that fails costs PRs a cold cache, not correctness. That is not worth a red ❌ on a service repo's default branch, so the job is now `continue-on-error` on push. PR runs are deliberately untouched: they keep failing loudly, because that ❌ is what makes the sticky advisory comment worth reading. The root cause is fixed separately in the action itself (blairham/go-pre-commit#33, released as v4.6.1 — `curl --retry` only covers transient HTTP responses and timeouts, never transport errors like a mid-transfer reset). This is the defense-in-depth half: even with retries, an infra step failing on a seed run should not gate anything. Bumping the pinned action version to v4.6.1 is a separate change. --- .github/workflows/pre-commit-advisory.yml | 13 +++++++++++++ AGENTS.md | 2 ++ 2 files changed, 15 insertions(+) diff --git a/.github/workflows/pre-commit-advisory.yml b/.github/workflows/pre-commit-advisory.yml index 645af8e..b4a0be0 100644 --- a/.github/workflows/pre-commit-advisory.yml +++ b/.github/workflows/pre-commit-advisory.yml @@ -79,6 +79,19 @@ jobs: # run, the default-branch scope is never populated, so every new # PR branch rebuilds hook envs from scratch on its first run. if: github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == format('refs/heads/{0}', github.event.repository.default_branch)) + # The push run seeds caches and gates nothing — its hook steps already + # end in `|| true` and the "reflect exit code" step below is + # pull_request-only, so it can never legitimately go red. It could still + # fail on an *infrastructure* step, and did: a transient + # `curl: (35) Recv failure: Connection reset by peer` while downloading + # the go-pre-commit release archive turned an otherwise-green k5s main + # into a red ❌ (k5s run 30964880559; a plain re-run went green with no + # change). A seed run failing costs PRs a cold cache, not correctness — + # that is not worth painting a service repo's default branch red. + # + # PR runs are unaffected: they keep failing loudly so engineers still see + # the ❌ that makes the advisory comment worth reading. + continue-on-error: ${{ github.event_name == 'push' }} steps: - uses: actions/checkout@v6 with: diff --git a/AGENTS.md b/AGENTS.md index 237298a..2eb8b8d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,6 +51,8 @@ After tagging `vX.Y.Z+`, `tag-config.yml` mints a GitHub App token (`BOOTST It posts a sticky PR comment with hook output and fails the job (so engineers see a ❌), but is **not** in any repo's required-checks list. Scoped to PR diff (`--from-ref`/`--to-ref`) so engineers only see violations they introduced. If you ever wire a setup step (e.g. another language toolchain) into the advisory workflow, gate it behind an input that defaults to `false` — matches the existing `setup-dotnet` / `setup-node-pnpm` pattern. +The **push-to-default-branch run is `continue-on-error`** — it only seeds caches (its hook steps end in `|| true`, and the exit-code-reflecting step is `pull_request`-only), so it gates nothing and must never redden a service repo's default branch. It previously could, via an infrastructure step rather than a hook: a transient `curl: (35) Recv failure` fetching the go-pre-commit release archive failed an otherwise-green k5s main. A failed seed costs PRs a cold cache, nothing more. Keep PR runs failing loudly — that ❌ is what makes the advisory comment worth reading. + ## Editing playbook - **Changing the docker matrix shape** in `discover-services/action.yml`: its docker-release consumer now lives only on the frozen `@pre-stevedore` tag (never retarget it) — pinned callers' `ci.yml` files still read the matrix output, so treat the shape as frozen until the stevedore migration completes.