From f3c75d1383656725dada87b7668b8fc5cbbf2835 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 22 Sep 2026 08:09:36 +0200 Subject: [PATCH] fix(release-notes-status): count Container image among the CI blocks The check that reports a CI-appended block destroyed by an overhaul looked for three sections: Installation, Verify your download and Software Bill of Materials. release-go-app.yml emits a fourth, `## Container image`, for every run that builds one -- the three rolling tags a consumer pulls. It was never in the list, so an overhaul that replaced the whole body destroyed it in silence and the script answered `ok`. Measured on netresearch/ldap-manager v1.7.0: the overhaul dropped both `## Container image` and `## Verify your download`, the check flagged only the second, and the release went out naming no image anywhere. The list moves into ci_block_sections() so the test reads what the check reads. A retyped copy in the test would agree with whatever the check does and stay green when a section is dropped; three assertions pin it instead, each verified by a mutation aimed at it -- removing "Container image" from the function reddens two, putting the literal list back in the loop reddens the third, and shellcheck stays clean. references/release-process.md gains the other half. The capture recipe it already documents only works BEFORE the first edit; afterwards the reflex is to copy the block out of the previous release, which is an output of the same pipeline and usually carries the same damage. The sibling comparison cannot catch that either: once a block is gone from every release on the line, nothing is left to compare against. The authority is the workflow that emits it. Learning-Id: retro-20260922-ci-block-container-image Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_013GWpRpyiM13Rh1NMQMBJo5 Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- .../references/release-process.md | 2 ++ .../scripts/release-notes-status.sh | 20 +++++++++++++++++-- .../tests/release-notes-status.test.sh | 19 ++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/skills/github-release/references/release-process.md b/skills/github-release/references/release-process.md index 4d77f08..0e93c8a 100644 --- a/skills/github-release/references/release-process.md +++ b/skills/github-release/references/release-process.md @@ -215,6 +215,8 @@ gh release edit vX.Y.Z --repo owner/repo --notes-file /tmp/final.md Then re-check the emitted verify commands are correct for a reusable-workflow build (`--signer-workflow`, not `--repo` alone). +**Capture the body BEFORE the first `gh release edit`, and never rebuild a lost block from a sibling release.** The capture above only works while the original body still exists; after the edit it is gone, and the reflex is to copy the block out of the previous release and rewrite its version strings. That previous release is an output of the same pipeline and usually carries the same damage — so the repair inherits it, and the check cannot object, because its comparison set is exactly those siblings: a block destroyed in every release on the line is invisible to `release-notes-status.sh` and the verdict reads `ok`. The authority is the workflow that emits the block. Read the `Compose release body` step of the orchestrator (`release-go-app.yml` around line 790) and reconstruct from it. Observed 2026-09-22 on `netresearch/ldap-manager` v1.7.0: the overhaul destroyed `## Container image` and `## Verify your download`, the checker flagged only the second, the first was restored from v1.6.0 — which had lost it to an earlier overhaul — and both releases went out without the image reference. + **Publish behaviour differs by orchestrator.** `release-go-app.yml` (go apps) publishes the release **directly** on tag push — not a draft — with all assets attached, so the overhaul edits a live release. `golib-create-release.yml` (go libraries) creates a **draft** that must be published manually with `gh release edit vX.Y.Z --draft=false` (a permitted flag) after CI finishes. Check `gh release view --json isDraft` to know which you have. Note: `gh release view --json isLatest` is NOT a valid field — query `gh api repos/OWNER/REPO/releases/latest` for the latest tag instead. #### Verify each publication claim before reporting the release done diff --git a/skills/github-release/scripts/release-notes-status.sh b/skills/github-release/scripts/release-notes-status.sh index a22e506..eb76f31 100755 --- a/skills/github-release/scripts/release-notes-status.sh +++ b/skills/github-release/scripts/release-notes-status.sh @@ -60,6 +60,22 @@ body_has() { # ... grep -q "$@" <<<"$text" } +# The sections the netresearch orchestrators append after `## Changes`, one per +# line. It is a function rather than a literal inside the loop so the test reads +# the same list the check reads, instead of a retyped copy that agrees with +# whatever the check happens to do. +# +# `release-go-app.yml` emits `## Container image` (the three rolling tags) when +# the run builds one, and `## Verify your download` always; the library and +# source-archive orchestrators emit `## Installation` and +# `## Software Bill of Materials`. Anything this list omits is destroyed by an +# overhaul in silence — `## Container image` was omitted here and a measured +# v1.7.0 release lost it while this script reported `ok`. +ci_block_sections() { + printf '%s\n' Installation "Container image" "Verify your download" \ + "Software Bill of Materials" +} + # Sourced by the test to exercise the helpers above; everything below needs gh. if [ "${BASH_SOURCE[0]}" != "${0}" ]; then return 0; fi @@ -139,11 +155,11 @@ check_one() { "[.[] | select(.draft==false) | select(.tag_name!=\"$tag\") | {t: .tag_name, b: .body}] | map(select(.t | ltrimstr(\"v\") | split(\".\")[0] == \"$major_of_tag\")) | map(.b) | join(\"\n\")" 2>/dev/null || true) - for s in Installation "Verify your download" "Software Bill of Materials"; do + while IFS= read -r s; do if body_has "$sibling" -F "## $s" && ! body_has "$body" -F "## $s"; then lost="$lost \"$s\"" fi - done + done <<< "$(ci_block_sections)" local next="ok" [ "$raw" = 1 ] || [ "$stub" = 1 ] && next="overhaul-notes" diff --git a/skills/github-release/scripts/tests/release-notes-status.test.sh b/skills/github-release/scripts/tests/release-notes-status.test.sh index 3b9adec..57d97d6 100755 --- a/skills/github-release/scripts/tests/release-notes-status.test.sh +++ b/skills/github-release/scripts/tests/release-notes-status.test.sh @@ -150,4 +150,23 @@ pipelines=$(grep -v '^[[:space:]]*#' "$(dirname "${BASH_SOURCE[0]}")/../release- | grep -c '| *grep -q' || true) check "no pattern test goes through a pipe" 0 "$pipelines" +# --- which CI blocks the overhaul check looks for ----------------------------- +# The list is read from the script, not retyped here: a copy would agree with +# whatever the check does and stay green when a section is dropped from it. +# `## Container image` is the section this pins. `release-go-app.yml` emits it +# for every run that builds a container, and it was absent from the check's list +# until a measured ldap-manager v1.7.0 overhaul destroyed it while this script +# answered `ok`. The sibling comparison cannot recover from that on its own: +# once a block is gone from every release on the line, no sibling carries it and +# the check has nothing left to compare against. +check "Container image is a CI block the check looks for" \ + 1 "$(ci_block_sections | grep -cxF 'Container image')" +check "every orchestrator section is listed" \ + 4 "$(ci_block_sections | grep -c .)" + +# Structural: the check must READ that list rather than carry its own copy. +literal=$(grep -c 'for s in Installation' \ + "$(dirname "${BASH_SOURCE[0]}")/../release-notes-status.sh" || true) +check "the loop reads ci_block_sections, not a retyped list" 0 "$literal" + exit $fail