fix: the smoke always runs, so a change to the smoke can verify itself (4323)
CI / lint (push) Successful in 4s
CI / extension-version (push) Successful in 4s
Build images / sign-extension (push) Successful in 4s
Build images / build-agent (push) Successful in 7s
Build images / build-ml (push) Successful in 7s
Build images / build-web (push) Successful in 5s
extension / lint (push) Successful in 18s
CI / frontend-build (push) Successful in 18s
CI / backend-lint-and-test (push) Successful in 33s
Build images / smoke-web (push) Successful in 26s
Build images / promote (push) Skipped
CI / integration (push) Successful in 2m10s

This workflow file is in no artifact's path set — correctly, since editing it
changes no shipped byte. But smoke-web was gated on build-web having
published something, so a commit touching ONLY the smoke moved no revision,
hit reuse, emitted no digest, and skipped the smoke. The one commit whose
purpose is changing this check was the one commit that could not run it.

Twice already: 5ca1058 added the egress sandbox and went green three times
with smoke-web SKIPPED; 7175ace fixed the bug hiding behind those greens
(#4319) and needed a manual force_build to exercise. Both relied on someone
remembering. It is also where the other historical skip lived — run 5290's
`if:` read `env`, which a job condition cannot see, so it evaluated empty and
skipped silently. Two skips, one expression. The expression goes.

The job now smokes whichever manifest is current: the digest this run built,
or — on a reuse hit — the one the channel tag already names, which the reuse
step resolves anyway to read its fc.revision label and now exports as
`published_digest`. Always a digest, never a tag (#4290). Kept separate from
`digest`, which the :c-<sha> repoint reads and must go on meaning "what this
run built". With neither available the job FAILS with a reason rather than
passing quietly.

Adding build.yml to WEB_PATHS would also make the smoke run, and would be
wrong: fc.revision means "the commit this artifact's shipped files last
changed in", so moving it for a CI edit makes the label lie and rebuilds
three images for a change none of them ship. The problem was never the
artifact's identity — it was when the guard runs.

Two more defects in the same block, both from 5ca1058, both found by run 7288
— which only existed because the force_build above ran the check a second
time:

  * `NET=smoke-noegress-$$` is not unique. The shell's pid is deterministic
    in this runner — every execution got 157 — so the second run died on
    "network with name smoke-noegress-157 already exists". A pid is unique
    among live processes, which is not unique over time.

  * The network's cleanup trap was destroyed before it could fire.
    `trap ... EXIT` REPLACES the previous handler, and the container's trap
    was installed further down, so every run leaked its network. Invisible in
    a passing run; it can only ever surface on the NEXT one.

Now one EXIT handler does both, armed beside the network and tolerant of an
empty CID so it covers failures before the container exists. Plus a sweep of
any network earlier runs leaked, which fails harmlessly on one still in use.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
2026-09-22 11:20:36 -04:00
co-authored by Claude Opus 5
parent 7175ace67a
commit 187b6d2cdf
+116 -18
View File
@@ -506,6 +506,10 @@ jobs:
# it by digest rather than by tag: a tag can move between the build and
# the smoke, and then the check reports on bytes nobody built here.
digest: ${{ steps.build.outputs.digest }}
# What the channel tag already names. This is what smoke-web checks on a
# reuse hit — deliberately NOT folded into `digest`, which the :c-<sha>
# repoint reads and which must keep meaning "what this run built" (#4290).
published_digest: ${{ steps.reuse.outputs.published_digest }}
# A plain `needs` — no `always()`. That expression existed to let a
# SKIPPED sign-extension through on a tag push while still blocking a
# FAILED one. With no tag trigger, sign-extension always runs, so the
@@ -808,6 +812,16 @@ jobs:
PUBLISHED=$(docker buildx imagetools inspect "$IMAGE:$T" \
--format '{{ index .Image.Config.Labels "fc.revision" }}' \
2>/dev/null || echo "")
# The manifest the channel tag names RIGHT NOW. Exported so the
# smoke has something to check on a reuse hit, when this job builds
# nothing and emits no digest of its own (#4323). Resolved here
# because this step is already resolving the tag to read its label —
# one lookup, one answer, rather than a second one that could name a
# different manifest if anything moved the tag in between.
PUBLISHED_DIGEST=$(docker buildx imagetools inspect "$IMAGE:$T" \
--format '{{ .Manifest.Digest }}' 2>/dev/null || echo "")
echo "published_digest=$PUBLISHED_DIGEST" >> "$GITHUB_OUTPUT"
echo "reuse: $IMAGE:$T carries fc.revision=${PUBLISHED:-<none>}; derived=$DERIVED"
if [ -z "$PUBLISHED" ] && docker buildx imagetools inspect "$IMAGE:$T" >/dev/null 2>&1; then
# The tag resolves but carries no readable label. Expected exactly
@@ -1172,9 +1186,29 @@ jobs:
# shape the refresh already has — per-channel candidate tags, promote
# learning its channel, and the :c-<sha> repoint moving after the gate.
# That is a redesign of the production publish path and is its own task.
if: >-
needs.build-web.outputs.candidate == 'true'
|| needs.build-web.outputs.digest != ''
#
# NO `if:` — this job always runs (#4323). It used to be gated on the
# build having published something, which skipped it on a reuse hit. That
# sounds like an optimisation and is a hole: this workflow file is in no
# artifact's path set (correctly — editing it changes no shipped byte), so
# a commit that touches ONLY the smoke moves no revision, hits reuse,
# emits no digest, and skips the smoke. The one commit whose purpose is
# changing this check was the one commit that could not run it.
#
# That is not hypothetical twice over. 5ca1058 added the egress sandbox
# and went green three times with this job SKIPPED. 7175ace fixed the
# bug that hid behind those greens (#4319) and needed a manual
# force_build to exercise at all. Both relied on someone remembering.
#
# It is also where the other historical failure lived: on run 5290 this
# expression read `env`, which a job `if:` cannot see, so it evaluated
# empty and skipped silently. Two skips, one expression. The expression
# goes.
#
# The cost is one pull and boot on a reuse-hit push, re-smoking bytes
# that were smoked when they were built. That is the price of a harness
# that tests itself, and it overlaps ci.yml's lanes, so little wall-clock
# moves. Not running is not the same as passing.
runs-on: python-ci
container:
image: git.fabledsword.com/bvandeusen/ci-python:3.14
@@ -1216,6 +1250,8 @@ jobs:
TOKEN: ${{ secrets.RELEASE_TOKEN }}
ACTOR: ${{ github.actor }}
BUILT_DIGEST: ${{ needs.build-web.outputs.digest }}
PUBLISHED_DIGEST: ${{ needs.build-web.outputs.published_digest }}
IS_CANDIDATE: ${{ needs.build-web.outputs.candidate }}
run: |
set -eux
# Service discovery mirrors ci.yml's integration lane: these jobs run
@@ -1245,14 +1281,32 @@ jobs:
fi
echo "$TOKEN" | docker login git.fabledsword.com -u "$ACTOR" --password-stdin
# A refresh publishes to the candidate tag; a push writes the
# channel tag directly and hands us its digest. Address the digest
# where we have one — it names the exact manifest this run built,
# which a tag stops doing the moment anything else moves it.
if [ -n "${BUILT_DIGEST:-}" ]; then
CANDIDATE="$IMAGE@$BUILT_DIGEST"
else
# WHAT GETS SMOKED, in order of preference — always a DIGEST, never
# a tag: a tag can move between the build and this job, and then the
# check reports on bytes nobody here decided to ship (#4290).
#
# 1. what this run built, when it built;
# 2. what the channel tag already names, on a reuse hit — the
# digest the reuse step resolved while reading its label. This
# is the case that makes the job able to verify a change to
# ITSELF (#4323), since such a change rebuilds nothing.
#
# A refresh always lands in (1); the candidate tag stays only as the
# last resort for one, and says so rather than being a silent else.
DIGEST="${BUILT_DIGEST:-}"
[ -n "$DIGEST" ] || DIGEST="${PUBLISHED_DIGEST:-}"
if [ -n "$DIGEST" ]; then
CANDIDATE="$IMAGE@$DIGEST"
elif [ "${IS_CANDIDATE:-}" = "true" ]; then
CANDIDATE="$IMAGE:refresh-candidate"
else
# Nothing built and nothing published. There is no artifact this
# job could honestly report on, so it fails rather than passing
# quietly — a guard with nothing to check is not a passing guard.
echo "smoke: FAILED — no image to smoke. build-web neither built" >&2
echo "smoke: one nor resolved a published digest, so there is" >&2
echo "smoke: nothing here to verify." >&2
exit 1
fi
docker pull "$CANDIDATE"
@@ -1284,9 +1338,55 @@ jobs:
# They keep their original network too — that is fine, since what
# must be offline is the APP container, and it is created with only
# this network.
NET=smoke-noegress-$$
# Named for the RUN, not for `$$`. The shell's pid is deterministic
# in this runner — every execution of this step got 157 — so `$$`
# produced one shared name, and the second run died on "network with
# name smoke-noegress-157 already exists". A pid is unique among
# LIVE processes, which is not the same as unique over time, and in
# a fresh container it is neither.
# The date fallback matters: if the runner does not set these,
# a literal default would put every run back on one shared name
# — the bug, with different letters.
NET="smoke-noegress-${GITHUB_RUN_ID:-$(date +%s)}-${GITHUB_RUN_ATTEMPT:-0}"
# Sweep anything an earlier run left behind. Needed because the
# cleanup below used to be destroyed before it could fire (see the
# trap note), so every execution leaked its network. `rm` on one
# still in use fails, and `|| true` keeps that harmless — so a
# concurrent run's network survives this.
docker network ls --filter name=^smoke-noegress- -q \
| while read -r stale; do
docker network rm "$stale" >/dev/null 2>&1 || true
done
docker network create --internal "$NET"
trap 'docker network rm "$NET" >/dev/null 2>&1 || true' EXIT
# ONE exit trap, for everything. `trap ... EXIT` REPLACES the
# previous handler rather than adding to it, so the network's own
# trap used to be silently discarded the moment the container's was
# installed further down — and the network was never removed. That
# is invisible in a passing run and only ever surfaces on the NEXT
# one, as a name collision.
#
# CID is empty until the app container exists, so this is safe to
# arm now and still covers a failure before that point.
CID=""
cleanup() {
rc=$?
if [ -n "$CID" ]; then
# The log ONLY on failure — a boot that never answered must fail
# with the reason visible rather than as a bare timeout (rule
# 156), while a green run has nothing to say.
[ $rc -eq 0 ] || docker logs "$CID" 2>&1 | tail -40
docker rm -f "$CID" >/dev/null 2>&1 || true
fi
docker network rm "$NET" >/dev/null 2>&1 || true
# Preserve the real status, which a trap ending on a successful
# `docker rm` would otherwise mask.
exit $rc
}
trap cleanup EXIT
docker network connect "$NET" "$PG"
docker network connect "$NET" "$RD"
# Re-read the addresses ON THIS NETWORK. The IPs discovered above
@@ -1344,13 +1444,11 @@ jobs:
# siblings directly and a published port would collide with
# whatever else the runner is hosting.
echo "smoke: web boots and answers /api/health"
# Assigning CID is all that is needed — the single EXIT trap armed
# beside the network already covers the container, and it checks CID
# for emptiness precisely so it can be installed before this line.
# Installing a second trap here is what used to discard the first.
CID=$(docker run -d $ENVOPTS "$CANDIDATE" web)
# Clean up the container however this ends, and dump its log ONLY
# on failure — a boot that never answers must fail with the reason
# visible rather than as a bare timeout (rule 156), while a green run
# has nothing to say. `exit $rc` preserves the real status, which a
# trap that ends on a successful `docker rm` would otherwise mask.
trap 'rc=$?; [ $rc -eq 0 ] || docker logs "$CID" 2>&1 | tail -40; docker rm -f "$CID" >/dev/null 2>&1 || true; exit $rc' EXIT
WEB_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$CID")
test -n "$WEB_IP"