Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions .github/workflows/bf-build-stable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,20 @@ jobs:
token: ${{ steps.app-token.outputs.token }}
persist-credentials: true

- name: Fetch patch manifest from brightfire/ci
# Pin the manifest used by this run: stage it to the working tree AND
# save a frozen copy under /tmp + record its sha256 to GITHUB_ENV. Every
# later step (parse, fingerprint gate, fingerprint writer) reads from
# the pinned copy, not origin/brightfire/ci, so concurrent pushes to
# brightfire/ci during build/test can't move the gate out from under us.
- name: Fetch patch manifest from brightfire/ci (pinned)
id: pin-manifest
run: |
git show origin/brightfire/ci:BRIGHTFIRE_PATCHES.md > BRIGHTFIRE_PATCHES.md
echo "Loaded patch manifest from brightfire/ci"
cp BRIGHTFIRE_PATCHES.md /tmp/BRIGHTFIRE_PATCHES.pinned.md
MANIFEST_SHA=$(sha256sum /tmp/BRIGHTFIRE_PATCHES.pinned.md | awk '{print $1}')
echo "Pinned manifest sha256: $MANIFEST_SHA"
echo "PINNED_MANIFEST_PATH=/tmp/BRIGHTFIRE_PATCHES.pinned.md" >> "$GITHUB_ENV"
echo "PINNED_MANIFEST_SHA256=$MANIFEST_SHA" >> "$GITHUB_ENV"

- name: Stage patch parser
run: cp scripts/bf/parse-patches.py /tmp/parse-patches.py
Expand Down Expand Up @@ -202,9 +212,13 @@ jobs:
id: release-check
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PATCHES_FILE: ${{ env.PATCHES_FILE }}
FORCE_RELEASE: ${{ inputs.force_release || 'false' }}
UPSTREAM_TAG_INPUT: ${{ inputs.upstream_tag || '' }}
# Pinned to the manifest staged at the top of the workflow so the
# gate compares against the manifest that actually drove the build,
# not whatever origin/brightfire/ci has advanced to since.
PINNED_MANIFEST_PATH: ${{ env.PINNED_MANIFEST_PATH }}
PINNED_MANIFEST_SHA256: ${{ env.PINNED_MANIFEST_SHA256 }}
run: /tmp/bf-build-stable/check-release-needed.sh

# Tarball is built unconditionally. `npm pack` is cheap (~10s) and
Expand Down
72 changes: 57 additions & 15 deletions scripts/bf/build-stable/check-release-needed.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,36 +26,78 @@
# - Latest release has no asset -> needs_release=true (backfill run)
# - gh release view fails (other) -> fail loudly (no silent skip)
#
# Manifest source (preferred): a pinned file path + pre-computed sha256
# staged by the workflow's "Fetch patch manifest from brightfire/ci (pinned)"
# step. Pinning matters because origin/brightfire/ci can advance between the
# moment the build started and the moment this gate runs — concurrent pushes
# happen during the ~30+ min build/test window, and several intervening
# `git fetch` calls update the remote tracking ref. Reading the moving ref
# here would gate (and fingerprint) on a manifest that did NOT drive this
# build, letting newly-registered patches sneak into a fingerprint that
# claims to represent only the older set.
#
# Fallback: read from a git ref (default origin/brightfire/ci). This keeps
# local invocations working and provides a graceful degradation if the
# workflow ever ran this script without staging the pinned copy.
#
# Inputs (env):
# GITHUB_REPOSITORY — owner/repo (set by GHA runtime)
# GITHUB_TOKEN — used implicitly by gh
# GITHUB_OUTPUT — path to GitHub Actions output file
# PATCHES_FILE — manifest path (default: BRIGHTFIRE_PATCHES.md)
# FORCE_RELEASE — "true" forces needs_release=true (from workflow_dispatch)
# UPSTREAM_TAG_INPUT — non-empty value forces needs_release=true (from
# workflow_dispatch upstream_tag input)
# GITHUB_REPOSITORY — owner/repo (set by GHA runtime)
# GITHUB_TOKEN — used implicitly by gh
# GITHUB_OUTPUT — path to GitHub Actions output file
# PINNED_MANIFEST_PATH — path to the staged pinned manifest; preferred
# source when present and readable
# PINNED_MANIFEST_SHA256 — sha256 of the pinned manifest, computed at
# staging time; used directly when set so the
# fingerprint writer and the gate share the
# exact same value byte-for-byte
# MANIFEST_REF — git ref fallback (default:
# origin/brightfire/ci); override only for
# local testing
# FORCE_RELEASE — "true" forces needs_release=true (from
# workflow_dispatch)
# UPSTREAM_TAG_INPUT — non-empty value forces needs_release=true
# (from workflow_dispatch upstream_tag input)
#
# Outputs (written to $GITHUB_OUTPUT):
# needs_release — "true" | "false"
# manifest_sha256 — sha256 of the current manifest
# manifest_sha256 — sha256 of the manifest that drove this build

set -euo pipefail

PATCHES_FILE="${PATCHES_FILE:-BRIGHTFIRE_PATCHES.md}"
MANIFEST_REF="${MANIFEST_REF:-origin/brightfire/ci}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Pin release checks to the built manifest

In the bf-build-stable workflow, the manifest that drives the build is copied and parsed near the start (.github/workflows/bf-build-stable.yml:70-86), but the workflow later refreshes origin/brightfire/* before this script runs (.github/workflows/bf-build-stable.yml:112). If brightfire/ci advances during the long build/test phase, this moving default reads the newer manifest instead of the manifest that produced the current stable tree, so the release gate can skip or write a fingerprint for patches that were not actually built. Pass a pinned ref/blob or the staged manifest hash from the parse step instead of rereading origin/brightfire/ci.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 9849462.

The race is real even with concurrency: cancel-in-progress: cancel signals aren't instant, and the workflow has an intervening git fetch origin '+refs/heads/brightfire/*:refs/remotes/origin/brightfire/*' step that advances origin/brightfire/ci mid-run. Worse, since the fingerprint writer also reads from the (re-read) manifest_sha256 output, a mid-build push would have made us publish a fingerprint claiming to represent a manifest the build never actually saw.

Fix: the "Fetch patch manifest" step now also stages a frozen copy to /tmp/BRIGHTFIRE_PATCHES.pinned.md and records its sha256 to GITHUB_ENV. check-release-needed.sh accepts PINNED_MANIFEST_PATH / PINNED_MANIFEST_SHA256 and prefers them; it falls back to the MANIFEST_REF git read (with a warning) for local invocations. Same sha now flows into both the gate comparison and the published fingerprint asset.

Verified all three branches manually (pinned + pre-sha, pinned + auto-sha, missing-pinned fallback). +70/-23 across the workflow and the script.

FORCE_RELEASE="${FORCE_RELEASE:-false}"
UPSTREAM_TAG_INPUT="${UPSTREAM_TAG_INPUT:-}"

if [ ! -f "$PATCHES_FILE" ]; then
echo "::error::Manifest not found: $PATCHES_FILE"
exit 2
fi
PINNED_MANIFEST_PATH="${PINNED_MANIFEST_PATH:-}"
PINNED_MANIFEST_SHA256="${PINNED_MANIFEST_SHA256:-}"

if [ -z "${GITHUB_OUTPUT:-}" ]; then
echo "::error::GITHUB_OUTPUT not set"
exit 2
fi

MANIFEST_SHA=$(sha256sum "$PATCHES_FILE" | awk '{print $1}')
if [ -n "$PINNED_MANIFEST_PATH" ] && [ -r "$PINNED_MANIFEST_PATH" ]; then
# Preferred path: pinned file + pre-computed sha.
if [ -n "$PINNED_MANIFEST_SHA256" ]; then
MANIFEST_SHA="$PINNED_MANIFEST_SHA256"
else
MANIFEST_SHA=$(sha256sum "$PINNED_MANIFEST_PATH" | awk '{print $1}')
fi
echo "Manifest source: pinned ($PINNED_MANIFEST_PATH)"
else
if [ -n "$PINNED_MANIFEST_PATH" ]; then
echo "::warning::PINNED_MANIFEST_PATH set but not readable ($PINNED_MANIFEST_PATH); falling back to $MANIFEST_REF"
fi
if ! git rev-parse --verify "$MANIFEST_REF:BRIGHTFIRE_PATCHES.md" >/dev/null 2>&1; then
echo "::error::BRIGHTFIRE_PATCHES.md not found at $MANIFEST_REF"
exit 2
fi
# Pipe git-show through sha256sum so the trailing newline is preserved
# byte-for-byte (sha must match `sha256sum BRIGHTFIRE_PATCHES.md` taken
# elsewhere, e.g. by write-build-fingerprint.sh).
MANIFEST_SHA=$(git show "$MANIFEST_REF:BRIGHTFIRE_PATCHES.md" | sha256sum | awk '{print $1}')
echo "Manifest source: $MANIFEST_REF (fallback)"
fi

echo "Current manifest sha256: $MANIFEST_SHA"
echo "manifest_sha256=$MANIFEST_SHA" >> "$GITHUB_OUTPUT"

Expand Down
Loading