From a6f594ade9ba07fe74335d8f8e1ea48f9059e131 Mon Sep 17 00:00:00 2001 From: scttbnsn <80784472+scttbnsn@users.noreply.github.com> Date: Fri, 21 Aug 2026 12:28:36 -0400 Subject: [PATCH 1/2] fix(workflows): close three shared-workflow defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit starchart-refresh: the documented `release: [published]` trigger never fires. GitHub suppresses workflow runs for events caused by GITHUB_TOKEN, and every consuming repo publishes its release with exactly that — portwing via GoReleaser, drydock via `gh release create`. A caller wired from this file's own example lints clean, reads as correctly configured, and refreshes nothing forever. That's the silent-success shape the committed-SVG rework existed to remove, reintroduced by the instructions for it. Example is now a workflow_dispatch the release cut fires, with the suppression and its two documented exceptions written down so the next person doesn't rederive the broken version. Found by the sockguard lane after three repos had been told to adopt it. main-is-released: an exact tag match alone was never the invariant. Any tag satisfied it, so one named `snapshot` or `latest` parked on a drifted main read as a pass. Now requires a release-shaped version. Prerelease detection moved off `case *-*`, which called `my-tag` a prerelease and would have accepted it under allow-prerelease. main-is-released: a promotion merges before its tag is pushed, so a run in that window reported drift that resolved itself seconds later. Three attempts with a tag refetch between them. It can't mask real drift — an untagged main is still untagged on the last attempt — and a failed refetch warns rather than passing. Verified by extracting the decision block and running it against real repositories: v1.7.4 and 1.7.4 pass, snapshot/latest/my-tag fail as malformed, v1.7.0-rc.2 fails as prerelease and passes under allow-prerelease, and allow-prerelease does not reopen the any-tag hole. 87 contract tests green. --- .../tests/main_is_released_contract_test.py | 34 ++++++++++++- .../tests/starchart_refresh_contract_test.py | 39 ++++++++++++--- .github/workflows/main-is-released.yml | 48 +++++++++++++++---- .github/workflows/starchart-refresh.yml | 39 ++++++++++++--- 4 files changed, 136 insertions(+), 24 deletions(-) diff --git a/.github/tests/main_is_released_contract_test.py b/.github/tests/main_is_released_contract_test.py index c9368b2..3a6fbc5 100644 --- a/.github/tests/main_is_released_contract_test.py +++ b/.github/tests/main_is_released_contract_test.py @@ -35,7 +35,7 @@ def test_the_invariant_is_an_exact_tag_match(self): # drifted main too. It may only be used to report inside the failure # branch, never in the condition that decides pass or fail — so the # decisive slice is the condition line, not the whole if-block. - decisive = workflow.split("if ! tag=", 1)[1].split("\n", 1)[0] + decisive = workflow.split('if tag="$(', 1)[1].split("\n", 1)[0] self.assertIn("--exact-match", decisive) self.assertNotIn("--abbrev=0", decisive) @@ -57,6 +57,38 @@ def test_shallow_checkout_would_break_the_measurement(self): workflow = self.read_workflow() self.assertIn("fetch-depth: 0", workflow) + def test_an_exact_match_alone_is_not_the_invariant(self): + """Any tag satisfies --exact-match, including a moving or descriptive + one. A tag literally named `snapshot` parked on a drifted main reads + as a clean pass, which is the exact failure this workflow exists to + catch. Found by the sockguard lane, 2026-08-21.""" + workflow = self.read_workflow() + + self.assertIn("[0-9]+\\.[0-9]+\\.[0-9]+", workflow) + self.assertIn("not a release version tag", workflow) + + # Prerelease detection keys on the hyphen AFTER the version, not any + # hyphen anywhere — the old `case $tag in *-*)` called `my-tag` a + # prerelease and would have accepted it under allow-prerelease. + self.assertNotIn('case "$tag" in', workflow) + self.assertIn("^v?[0-9]+\\.[0-9]+\\.[0-9]+-", workflow) + + def test_the_merge_to_tag_window_is_retried_not_reported_as_drift(self): + """A promotion merges before its tag is pushed. A run landing in that + window sees an untagged main and reports drift that resolves itself + seconds later, which trains people to ignore the one check whose job + is being noticed.""" + workflow = self.read_workflow() + + self.assertIn("for attempt in 1 2 3", workflow) + self.assertIn("git fetch --tags --force", workflow) + self.assertIn("sleep 20", workflow) + + # The retry must not become a way to pass. A failed refetch is warned + # about and the loop still decides on the refs it has. + self.assertIn("::warning::could not refetch tags", workflow) + self.assertNotIn("exit 0", workflow) + def test_a_prerelease_on_main_fails_by_default(self): """A release candidate on the default branch is the exact drift this exists to catch: drydock's main sat on v1.7.0-rc.2.""" diff --git a/.github/tests/starchart_refresh_contract_test.py b/.github/tests/starchart_refresh_contract_test.py index 8fb137f..8962cf7 100644 --- a/.github/tests/starchart_refresh_contract_test.py +++ b/.github/tests/starchart_refresh_contract_test.py @@ -173,17 +173,42 @@ def test_both_themes_are_written_and_committed_together(self): # Both derivations strip a .svg suffix, so the input has to have one. self.assertIn("!out.endsWith('.svg')", workflow) - def test_the_documented_trigger_is_the_release_cut_not_a_cron(self): - """A committed artifact refreshed on a schedule mutates underneath a - tag, which is what 'main is the released version' forbids.""" + def test_the_documented_trigger_is_a_dispatch_not_a_cron_or_a_release(self): + """Two ways to get this wrong, and the second one looks right. + + A cron mutates a committed artifact underneath a tag, which 'main is + the released version' forbids. And `release: [published]` never fires + at all: GitHub suppresses workflow runs for events caused by + GITHUB_TOKEN, which is what every repo here publishes releases with, + so a caller wired that way is green everywhere and refreshes nothing. + This file told three repos to do exactly that on 2026-08-21 before the + sockguard lane caught it, so the example is pinned by a test now.""" workflow = self.read_workflow() example = workflow.split("# on:\n", 1)[1].split("# permissions:", 1)[0] - self.assertIn("release:", example) - self.assertIn("types: [published]", example) + self.assertIn("workflow_dispatch:", example) self.assertIn('# accent: "#49bcfb"', workflow) - self.assertNotIn("cron", example) - self.assertNotIn("schedule:", example) + for dead in ("release:", "types: [published]", "cron", "schedule:"): + self.assertNotIn(dead, example) + + def test_the_suppression_trap_is_documented_not_just_avoided(self): + """Removing the bad example only stops it being copied from here. The + reason has to travel with it, or the next person reaches for the + release trigger from first principles and it fails the same silent + way.""" + workflow = self.read_workflow() + + for expected in ( + "GITHUB_TOKEN", + "gh workflow run", + "workflow_dispatch` and", + "repository_dispatch", + ): + self.assertIn(expected, workflow) + + # The failure mode named, so it reads as a trap rather than a + # preference: wired that way it lints clean and never runs. + self.assertIn("refreshes nothing", workflow) def test_the_embedded_renderer_names_its_source(self): """The same renderer exists here and in ops. Hand-copying is how they diff --git a/.github/workflows/main-is-released.yml b/.github/workflows/main-is-released.yml index c4bb47e..efe1584 100644 --- a/.github/workflows/main-is-released.yml +++ b/.github/workflows/main-is-released.yml @@ -73,20 +73,50 @@ jobs: exit 1 fi - if ! tag="$(git describe --exact-match --tags HEAD 2>/dev/null)"; then + # A promotion merges before its tag is pushed, so a run landing in + # that window sees an untagged main and reports drift that resolves + # itself seconds later. Refetch and retry before believing it. This + # cannot mask real drift: a genuinely untagged main is still + # untagged on the last attempt. + tag="" + for attempt in 1 2 3; do + if tag="$(git describe --exact-match --tags HEAD 2>/dev/null)"; then + break + fi + tag="" + [ "$attempt" -eq 3 ] && break + echo "main is untagged on attempt ${attempt}; refetching tags in case a promotion is mid-cut" + sleep 20 + # A failed refetch is reported, never swallowed into a pass: the + # loop still decides on whatever refs we actually have. + git fetch --tags --force --quiet origin \ + || echo "::warning::could not refetch tags on attempt ${attempt}; the verdict below uses the refs from checkout" + done + + if [ -z "$tag" ]; then latest="$(git describe --tags --abbrev=0 HEAD 2>/dev/null || echo '')" ahead="$(git rev-list --count "${latest}..HEAD" 2>/dev/null || echo '?')" echo "::error::main is not a tagged release. Newest reachable tag is ${latest}, and main is ${ahead} commit(s) past it. Either cut a release or move the unshipped work to a dev branch." >&2 exit 1 fi - case "$tag" in - *-*) - if [ "$ALLOW_PRERELEASE" != "true" ]; then - echo "::error::main points at prerelease ${tag}. Prereleases belong on the dev branch; main carries what users actually run." >&2 - exit 1 - fi - echo "::warning::main points at prerelease ${tag}, accepted because allow-prerelease is set" ;; - esac + # An exact match alone is not the invariant. Any tag satisfies it, + # including one literally named "snapshot" or "latest" parked on a + # drifted main — which reads as a pass and is the failure this + # workflow exists to catch. Require a release-shaped tag. + if ! printf '%s' "$tag" | grep -Eq '^v?[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$'; then + echo "::error::main points at ${tag}, which is not a release version tag (expected vMAJOR.MINOR.PATCH). A moving or descriptive tag satisfies an exact-match check while main carries unshipped work." >&2 + exit 1 + fi + + # Prerelease is the part AFTER the version, so match on that rather + # than on any hyphen anywhere in the tag. + if printf '%s' "$tag" | grep -Eq '^v?[0-9]+\.[0-9]+\.[0-9]+-'; then + if [ "$ALLOW_PRERELEASE" != "true" ]; then + echo "::error::main points at prerelease ${tag}. Prereleases belong on the dev branch; main carries what users actually run." >&2 + exit 1 + fi + echo "::warning::main points at prerelease ${tag}, accepted because allow-prerelease is set" + fi echo "main is released at ${tag}" diff --git a/.github/workflows/starchart-refresh.yml b/.github/workflows/starchart-refresh.yml index 3caaf04..a091eb0 100644 --- a/.github/workflows/starchart-refresh.yml +++ b/.github/workflows/starchart-refresh.yml @@ -8,8 +8,6 @@ name: Star Chart Refresh # Callers declare their own triggers and pin this file by full commit SHA: # # on: -# release: -# types: [published] # workflow_dispatch: # permissions: {} # jobs: @@ -18,13 +16,40 @@ name: Star Chart Refresh # contents: write # uses: CodesWhat/.github/.github/workflows/starchart-refresh.yml@ # with: -# branch: dev/v1.7 +# branch: ${{ github.ref_name }} # accent: "#49bcfb" # -# The trigger is the release cut, not a cron. A committed artifact refreshed -# on a schedule mutates underneath a tag, which is exactly what "main is the -# released version" forbids. Regenerating at the cut means the chart in a -# released README is as of that release. +# The refresh belongs to the release cut, not to a cron. A committed artifact +# refreshed on a schedule mutates underneath a tag, which is exactly what +# "main is the released version" forbids. +# +# DO NOT trigger it with `release: [published]`. That looks right and never +# runs. GitHub suppresses workflow runs for events caused by GITHUB_TOKEN, and +# every repo here publishes its release with that credential — GoReleaser with +# `GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}` (portwing release.yml) or +# `gh release create` with `GH_TOKEN: ${{ github.token }}` (drydock +# release-cut.yml). A caller wired that way reads as correctly configured, is +# green in every lint, and refreshes nothing, forever. Found 2026-08-21 by the +# sockguard lane after this file had already told three repos to do it. +# +# The release-cut workflow dispatches this one instead: +# +# - name: Dispatch starchart refresh +# env: +# GH_TOKEN: ${{ github.token }} +# run: gh workflow run starchart.yml --ref "$BRANCH" +# +# That works with no new credential because `workflow_dispatch` and +# `repository_dispatch` are the two documented exceptions to the suppression. +# The dispatch step must fail loudly rather than `|| true`: by the time it +# runs the release is already published, so a swallowed error is the same +# silent-success shape this whole workflow exists to remove. +# +# Prefer dispatching BEFORE the tag is cut where the flow allows it, so the +# released README ships the chart it claims to. Dispatching after publish is +# an accepted tradeoff — the chart then lands on the dev branch and main's +# copy is one cut stale — because the requirement is that regeneration is +# tied to the cut rather than to wall-clock time, and that holds either way. # # The generator is embedded rather than checked out from a second repository # so that the caller's SHA pin covers every line of behaviour, with nothing From 74db5a0ed3f473d3e179aae3593f8d99e4f5bc11 Mon Sep 17 00:00:00 2001 From: scttbnsn <80784472+scttbnsn@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:50:52 -0400 Subject: [PATCH 2/2] fix(workflows): correct two overstated claims CodeRabbit caught MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refetch warning said the verdict uses the refs from checkout. It might not: attempt 1 can succeed and attempt 2 fail, and a failed fetch can leave some refs updated. Now says the refs currently available on the runner, which is what's actually true. 'The two documented exceptions to the suppression' was an overclaim. pull_request with opened/synchronize/reopened is a third — it creates a run in an approval-required state rather than being suppressed. workflow_dispatch and repository_dispatch are the two that fire UNATTENDED, which is the property a release cut actually needs, so the comment now says that instead. --- .github/workflows/main-is-released.yml | 2 +- .github/workflows/starchart-refresh.yml | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main-is-released.yml b/.github/workflows/main-is-released.yml index efe1584..361b473 100644 --- a/.github/workflows/main-is-released.yml +++ b/.github/workflows/main-is-released.yml @@ -90,7 +90,7 @@ jobs: # A failed refetch is reported, never swallowed into a pass: the # loop still decides on whatever refs we actually have. git fetch --tags --force --quiet origin \ - || echo "::warning::could not refetch tags on attempt ${attempt}; the verdict below uses the refs from checkout" + || echo "::warning::could not refetch tags on attempt ${attempt}; the verdict below uses the refs currently available on the runner" done if [ -z "$tag" ]; then diff --git a/.github/workflows/starchart-refresh.yml b/.github/workflows/starchart-refresh.yml index a091eb0..e593394 100644 --- a/.github/workflows/starchart-refresh.yml +++ b/.github/workflows/starchart-refresh.yml @@ -40,7 +40,11 @@ name: Star Chart Refresh # run: gh workflow run starchart.yml --ref "$BRANCH" # # That works with no new credential because `workflow_dispatch` and -# `repository_dispatch` are the two documented exceptions to the suppression. +# `repository_dispatch` are the two dispatch events that always create a run +# even when GITHUB_TOKEN caused them. They are not the only exceptions — +# `pull_request` with opened/synchronize/reopened creates a run in an +# approval-required state rather than being suppressed outright — but they +# are the two that fire unattended, which is what a release cut needs. # The dispatch step must fail loudly rather than `|| true`: by the time it # runs the release is already published, so a swallowed error is the same # silent-success shape this whole workflow exists to remove.