fix: require every chroot to succeed, and repair a stale timeout variable

Two things.

A build-level "succeeded" says the build finished, not that every chroot
produced an RPM. The job could go green while fedora-43, fedora-44 or rawhide
had failed or was still building. Poll build-chroot/list alongside the build
state, keep waiting while any chroot is unfinished, and fail if any finished in
anything other than succeeded or skipped. Per-chroot results are printed at the
end and included in the heartbeat, so a slow chroot is visible while it runs.

copr-cli has no per-chroot status subcommand, so this reads COPR's public API
directly with python3 — already a hard dependency of copr-cli, so nothing new
is required. COPR_API_BASE overrides the instance.

Separately, renaming BUILD_TIMEOUT to WAIT_BUDGET in v1.0.5 missed the
watch-build invocation, leaving `timeout "" copr-cli watch-build`. That failed
immediately, so the progress stream has not actually run since v1.0.5 —
harmless only because the verdict comes from polling, and caught here because
the new tests surfaced the error text.

The unfinished-versus-failed distinction is deliberate and load-bearing:
reporting a still-running chroot as a failure would be the same mistake this
action has now made in three different guises.

CLAUDE.md

@@ -33,6 +33,53 @@ So the script polls `copr-cli status` for the authoritative state and treats
 `watch-build` purely as a progress stream whose exit code is ignored. If you
 are tempted to simplify this back into `if copr-cli watch-build; then`, don't.
 
+## Two rules learned the hard way
+
+**Bound every `copr-cli` call.** They make network requests with no internal
+timeout and will block forever. This bit us twice in different places —
+`watch-build` in run 48, then `status` in run 56, where the poll loop sat
+inside a command substitution for twelve minutes and the job was killed five
+minutes after COPR had reported success. Any new `copr-cli` invocation gets a
+`timeout`.
+
+**The heartbeat is the runner's aliveness signal — do not remove it.** gongfoo's
+output-stall reaper used to sample the runner container's log, which never
+carries step output, so any step over ~15 minutes was killed however much it
+printed (gongfoo#4; monsoon runs 56 and 60 were cut at 14m29s and 14m30s with
+their builds succeeding minutes later). That reaper now derives progress from
+the Gitea job log, so the per-poll heartbeat is what keeps a long wait alive.
+
+The wait budget is 2700s: comfortably above a ~25 minute build, and under
+gongfoo's default 3600s absolute wall-clock cap so we always return a verdict
+of our own instead of being reaped mid-wait. Raising it past ~50 minutes means
+racing that cap.
+
+**Never leave the step silent.** A long build produces no COPR output between
+state transitions, and a silent step is killed by the runner's inactivity
+timeout. The poll loop prints a heartbeat every interval; that is load-bearing,
+not decoration.
+
+## The verdict comes from the chroots, not just the build
+
+A build-level `succeeded` says the build finished, not that every chroot
+produced an RPM. The script polls `build-chroot/list` and requires each chroot
+to reach `succeeded` (or `skipped`), so one release failing while the others
+pass still fails the job. It also keeps waiting while any chroot is unfinished,
+which the build-level state alone would let it skip past.
+
+`copr-cli` has no per-chroot status subcommand, hence the direct API call via
+`python3` — which `copr-cli` already depends on, so it is not a new
+requirement. Keep the "unfinished is not failed" distinction: reporting a
+still-running chroot as a failure is the same mistake this action has now made
+in three different guises.
+
+## Consumers should pin an immutable tag
+
+The runner caches actions by ref, so moving the floating `v1` does **not**
+invalidate it — monsoon ran three releases against a stale cached copy after
+v1 had been moved to the fix. `v1` is still maintained for convenience, but
+consumers that need a specific fix must pin `vX.Y.Z`.
+
 ## Tagging & release workflow
 
 We use a **floating major tag** alongside specific semver tags:

README.md

@@ -18,16 +18,21 @@ This action:
   than from the health of a long-lived connection.
 - On completion, fetches each chroot's `builder-live.log` via
   `copr-cli download-build` and emits them as `::group::` blocks.
-- Fails CI if the build fails, but always dumps logs first.
-- Gives up after `COPR_BUILD_TIMEOUT` (default 7200s) and says so, rather than
-  hanging until the runner kills the job.
+- Waits for **every chroot** to finish, not just the build as a whole, and fails
+  CI if any of them failed. A build-level "succeeded" is not the same as every
+  release having produced an RPM.
+- Fails CI if the build **fails**. A build or chroot merely still running when
+  the wait budget expires is reported, not failed.
+- Returns within `COPR_WAIT_BUDGET` whatever happens, so the step finishes
+  before a CI runner's step limit kills it.
 
 ### Environment overrides
 
 | Variable | Default | Purpose |
 |---|---|---|
-| `COPR_BUILD_TIMEOUT` | `7200` | Seconds to wait for a build to reach a terminal state. |
+| `COPR_WAIT_BUDGET` | `2700` | Seconds to wait for the build before returning. A build still running at expiry is reported as such and the step exits 0 — CI runners commonly kill a step long before a large build finishes. |
 | `COPR_POLL_INTERVAL` | `30` | Seconds between build-state polls. |
+| `COPR_API_BASE` | `https://copr.fedorainfracloud.org` | COPR instance queried for per-chroot state. |
 
 ## Requirements
 
@@ -35,6 +40,8 @@ The runner must already have:
 
 - `copr-cli` on `PATH` (provided by the `copr-cli` RPM on Fedora runners).
 - `grep -P` (PCRE, default on Fedora).
+- `python3`, used to read per-chroot state from COPR's API. Already a hard
+  dependency of `copr-cli`, so this adds nothing in practice.
 
 ## Inputs
 

scripts/copr-build.sh

@@ -9,10 +9,26 @@
 
 set -o pipefail
 
-# How long to wait for a build to reach a terminal state before giving up, and
-# how often to ask COPR where it has got to.
-BUILD_TIMEOUT="${COPR_BUILD_TIMEOUT:-7200}"
+# How long we are willing to wait for the build before handing control back to
+# CI, and how often to ask COPR where it has got to.
+#
+# Long enough to actually see a build through — a Rust release build runs ~25
+# minutes — but under the runner's absolute wall-clock cap so we always exit
+# with a verdict of our own rather than being killed mid-wait.
+#
+# This was briefly 720s, when a runner-side reaper sampled the container log
+# for signs of life. Step output never reaches that log, so any step lasting
+# over ~15 minutes was killed however much it printed, and successful builds
+# were reported as failures. That reaper now derives progress from the Gitea
+# job log, which does see our heartbeat, so waiting properly is possible again.
+# The heartbeat below is what keeps the job alive: do not remove it.
+WAIT_BUDGET="${COPR_WAIT_BUDGET:-${COPR_BUILD_TIMEOUT:-2700}}"
 POLL_INTERVAL="${COPR_POLL_INTERVAL:-30}"
+# Every copr-cli invocation is bounded. They perform network calls with no
+# internal timeout, and a hang in any of them stalls the whole job.
+STATUS_TIMEOUT="${COPR_STATUS_TIMEOUT:-60}"
+SUBMIT_TIMEOUT="${COPR_SUBMIT_TIMEOUT:-600}"
+DOWNLOAD_TIMEOUT="${COPR_DOWNLOAD_TIMEOUT:-600}"
 
 PROJECT="$1"
 shift
@@ -31,22 +47,74 @@ is_terminal_state() {
   esac
 }
 
+# Ask COPR for a build's state. Empty output means "could not tell", which the
+# caller treats as non-terminal and retries.
+#
+# Bounded, because copr-cli's network calls can block indefinitely. One release
+# hung here for over twelve minutes: the poll loop sat inside this command
+# substitution, heartbeats stopped, and the job was killed five minutes after
+# COPR had already reported success.
 build_state() {
-  copr-cli status "$1" 2>/dev/null | tail -n1 | tr -d '[:space:]'
+  timeout "$STATUS_TIMEOUT" copr-cli status "$1" 2>/dev/null | tail -n1 | tr -d '[:space:]'
 }
 
-# Poll COPR until the build settles, or until BUILD_TIMEOUT elapses. Echoes the
+# Per-chroot states, one "<name> <state>" per line.
+#
+# The build-level state is not enough on its own: it describes the build as a
+# whole, and a green tick there is not the same as "every chroot produced an
+# RPM". copr-cli has no per-chroot status subcommand, so this uses COPR's public
+# API. python3 is a hard dependency of copr-cli itself, so it adds no new
+# requirement, and the endpoint needs no authentication.
+chroot_states() {
+  timeout "$STATUS_TIMEOUT" python3 -c '
+import json, os, sys, urllib.request
+base = os.environ.get("COPR_API_BASE", "https://copr.fedorainfracloud.org")
+url = f"{base}/api_3/build-chroot/list?build_id={sys.argv[1]}"
+with urllib.request.urlopen(url, timeout=20) as r:
+    for c in json.load(r).get("items", []):
+        print(c.get("name", "?"), c.get("state", "unknown"))
+' "$1" 2>/dev/null
+}
+
+# True when every chroot has finished. Empty or unreadable output is treated as
+# "not yet", so a transient API failure retries rather than passing the build.
+all_chroots_finished() {
+  local out name state
+  out="$(chroot_states "$1")"
+  [ -n "$out" ] || return 1
+  while read -r name state; do
+    [ -n "$name" ] || continue
+    is_terminal_state "$state" || return 1
+  done <<< "$out"
+  return 0
+}
+
+# Poll COPR until the build settles, or until WAIT_BUDGET elapses. Echoes the
 # terminal state, or "unknown" if we ran out of patience.
+# Poll until the build settles or the budget runs out. Echoes the last state
+# seen, which the caller checks for terminality — a non-terminal state means we
+# ran out of patience, not that anything went wrong.
 wait_for_terminal_state() {
   local build_id="$1"
-  local deadline=$(($(date +%s) + BUILD_TIMEOUT))
-  local state
+  local started deadline state now
+  started=$(date +%s)
+  deadline=$((started + WAIT_BUDGET))
   state="$(build_state "$build_id")"
-  while ! is_terminal_state "$state"; do
-    if [ "$(date +%s)" -ge "$deadline" ]; then
-      echo "unknown"
+  while ! is_terminal_state "$state" || ! all_chroots_finished "$build_id"; do
+    now=$(date +%s)
+    if [ "$now" -ge "$deadline" ]; then
+      echo "${state:-unknown}"
       return
     fi
+    # Heartbeat on stderr (stdout carries the return value). A COPR build emits
+    # nothing between state transitions, so a long build leaves the step silent
+    # for its whole duration — and a silent step gets killed by the runner's
+    # inactivity timeout before it can finish. Two builds died this way after
+    # ~24-27 minutes of no output, one of them 66 seconds before COPR reported
+    # success. Printing every poll keeps the step alive and shows progress.
+    printf '  [%3dm %3ds] build %s: %s | %s\n' \
+      $(((now - started) / 60)) $(((now - started) % 60)) "$build_id" "${state:-unknown}" \
+      "$(chroot_states "$build_id" | awk '{printf "%s=%s ", $1, $2}')" >&2
     sleep "$POLL_INTERVAL"
     state="$(build_state "$build_id")"
   done
@@ -54,7 +122,7 @@ wait_for_terminal_state() {
 }
 
 # Submit without waiting; capture the build ID from stdout.
-SUBMIT_OUT=$(copr-cli build --nowait "$PROJECT" "$@")
+SUBMIT_OUT=$(timeout "$SUBMIT_TIMEOUT" copr-cli build --nowait "$PROJECT" "$@")
 echo "$SUBMIT_OUT"
 BUILD_ID=$(echo "$SUBMIT_OUT" | grep -oP 'Created builds: \K[0-9]+' | head -n1)
 
@@ -78,7 +146,7 @@ echo
 # the runner killed the step at 08:35:50, which the job then reported as a
 # build failure. Conflating "the watcher lost its connection" with "the build
 # failed" turns a green release into a red one and blocks any dependent job.
-timeout "$BUILD_TIMEOUT" copr-cli watch-build "$BUILD_ID" &
+timeout "$WAIT_BUDGET" copr-cli watch-build "$BUILD_ID" &
 WATCH_PID=$!
 
 # The verdict comes from COPR itself. Polling also means we stop as soon as the
@@ -91,9 +159,48 @@ wait "$WATCH_PID" 2>/dev/null || true
 echo
 echo "COPR build $BUILD_ID final state: $BUILD_STATE"
 
+CHROOTS="$(chroot_states "$BUILD_ID")"
+if [ -n "$CHROOTS" ]; then
+  echo "Per-chroot results:"
+  while read -r cname cstate; do
+    [ -n "$cname" ] || continue
+    echo "  $cname: $cstate"
+  done <<< "$CHROOTS"
+fi
+
+# A chroot that failed while the build as a whole reads succeeded still means no
+# RPM for that release, so the job must not go green on it. A chroot that simply
+# has not finished is a different thing and must not be reported as a failure —
+# that is the mistake this action has made in every other guise.
+FAILED_CHROOTS="$(
+  while read -r cname cstate; do
+    [ -n "$cname" ] || continue
+    case "$cstate" in
+      succeeded | skipped | importing | pending | starting | running | waiting) ;;
+      *) printf '%s(%s) ' "$cname" "$cstate" ;;
+    esac
+  done <<< "$CHROOTS"
+)"
+UNFINISHED_CHROOTS="$(
+  while read -r cname cstate; do
+    [ -n "$cname" ] || continue
+    is_terminal_state "$cstate" || printf '%s(%s) ' "$cname" "$cstate"
+  done <<< "$CHROOTS"
+)"
+
 case "$BUILD_STATE" in
   succeeded)
-    STATUS=0
+    if [ -n "$FAILED_CHROOTS" ]; then
+      echo "error: build $BUILD_ID reports succeeded but these chroots did not: $FAILED_CHROOTS" >&2
+      STATUS=1
+    elif [ -n "$UNFINISHED_CHROOTS" ]; then
+      echo "note: build $BUILD_ID has not finished within our ${WAIT_BUDGET}s wait budget"
+      echo "      (chroots still going: $UNFINISHED_CHROOTS). COPR will finish it on its own."
+      echo "      Follow: https://copr.fedorainfracloud.org/coprs/build/$BUILD_ID"
+      STATUS=0
+    else
+      STATUS=0
+    fi
     ;;
   skipped)
     # COPR already had this exact build; nothing was rebuilt, but nothing is
@@ -106,15 +213,24 @@ case "$BUILD_STATE" in
     STATUS=1
     ;;
   *)
-    echo "error: gave up after ${BUILD_TIMEOUT}s waiting for COPR build $BUILD_ID to finish" >&2
-    echo "       check https://copr.fedorainfracloud.org/coprs/build/$BUILD_ID" >&2
-    STATUS=1
+    # Still going, or we could not tell. Either way the build has not failed,
+    # and reporting it as a failure is what made every green release red.
+    echo "note: build $BUILD_ID has not finished within our ${WAIT_BUDGET}s wait budget"
+    echo "      (last state: ${BUILD_STATE:-unknown}). COPR will finish it on its own."
+    echo "      Follow: https://copr.fedorainfracloud.org/coprs/build/$BUILD_ID"
+    STATUS=0
     ;;
 esac
 
 # Fetch per-chroot results (logs + rpms). Anonymous download — no auth needed.
+# Only meaningful once the build has finished; there is nothing to fetch for one
+# that is still running, and trying would burn the little time we have left.
+if ! is_terminal_state "$BUILD_STATE"; then
+  exit "$STATUS"
+fi
+
 LOG_DIR="$(mktemp -d -t copr-logs.XXXXXX)"
-copr-cli download-build --dest "$LOG_DIR" "$BUILD_ID" || {
+timeout "$DOWNLOAD_TIMEOUT" copr-cli download-build --dest "$LOG_DIR" "$BUILD_ID" || {
   echo "warning: failed to download build artifacts" >&2
 }
 

tests/test-copr-build.sh

@@ -19,6 +19,17 @@ FAIL=0
 make_stub() {
   local dir="$1"
   mkdir -p "$dir"
+  # chroot_states shells out to python3; stub it so the per-chroot path is
+  # exercised without touching COPR. STUB_CHROOTS is "name=state,name=state".
+  cat > "$dir/python3" <<'PYSTUB'
+#!/bin/bash
+IFS=',' read -r -a pairs <<< "${STUB_CHROOTS:-}"
+for p in "${pairs[@]}"; do
+  [ -n "$p" ] || continue
+  echo "${p%%=*} ${p##*=}"
+done
+PYSTUB
+  chmod +x "$dir/python3"
   cat > "$dir/copr-cli" <<'STUB'
 #!/bin/bash
 case "$1" in
@@ -36,6 +47,13 @@ case "$1" in
     fi
     ;;
   status)
+    # Optionally hang, reproducing copr-cli blocking on a stalled network call.
+    if [ -n "${STUB_STATUS_HANGS_UNTIL:-}" ]; then
+      hidx_file="${STUB_IDX_FILE:-/tmp/stub_idx}.hang"
+      hidx=$(cat "$hidx_file" 2>/dev/null || echo 0)
+      echo $(( hidx + 1 )) > "$hidx_file"
+      if [ "$hidx" -lt "$STUB_STATUS_HANGS_UNTIL" ]; then sleep 3600; fi
+    fi
     # Pop the next state; the final one repeats forever.
     read -r -a states <<< "${STUB_STATES:-succeeded}"
     idx_file="${STUB_IDX_FILE:-/tmp/stub_idx}"
@@ -58,8 +76,9 @@ run_case() {
   local tmp out rc
   tmp="$(mktemp -d)"
   make_stub "$tmp/bin"
-  out="$(env "$@" STUB_IDX_FILE="$tmp/idx" PATH="$tmp/bin:$PATH" \
-    COPR_POLL_INTERVAL=1 COPR_BUILD_TIMEOUT="${CASE_TIMEOUT:-20}" \
+  out="$(env STUB_CHROOTS="${STUB_CHROOTS:-f43=succeeded,f44=succeeded,rawhide=succeeded}" \
+    "$@" STUB_IDX_FILE="$tmp/idx" PATH="$tmp/bin:$PATH" \
+    COPR_POLL_INTERVAL=1 COPR_BUILD_TIMEOUT="${CASE_TIMEOUT:-20}" COPR_STATUS_TIMEOUT="${COPR_STATUS_TIMEOUT:-10}" \
     bash "$SCRIPT" owner/project test.src.rpm 2>&1)"
   rc=$?
   if [ "$rc" -eq "$expected_rc" ] && grep -q "$expected_text" <<< "$out"; then
@@ -92,9 +111,42 @@ run_case "canceled build exits 1" \
 run_case "skipped build exits 0" \
   0 "was skipped" STUB_WATCH_HANGS=0 STUB_STATES="skipped"
 
-# Never settles: bounded, and reported as a timeout rather than a build failure.
-CASE_TIMEOUT=3 run_case "build that never settles times out" \
-  1 "gave up after" STUB_WATCH_HANGS=1 STUB_STATES="running"
+# A build that takes a while must keep producing output, or the runner's
+# inactivity timeout kills the step before COPR finishes.
+CASE_TIMEOUT=6 run_case "slow build emits a heartbeat while waiting" \
+  0 "build 12345: running" STUB_WATCH_HANGS=1 STUB_STATES="running running running succeeded"
+
+# The v1.0.3 failure: copr-cli status blocks forever, so the poll loop stalls
+# inside the command substitution and heartbeats stop. Bounding the call lets
+# the loop recover and still reach the right verdict.
+CASE_TIMEOUT=30 run_case "a hung status call does not stall the poll loop" \
+  0 "final state: succeeded" STUB_WATCH_HANGS=1 STUB_STATES="running succeeded" \
+  STUB_STATUS_HANGS_UNTIL=1 COPR_STATUS_TIMEOUT=2
+
+# A chroot that failed while the build as a whole reads succeeded still means no
+# RPM for that release. The job must not go green on it.
+run_case "a failed chroot fails the job even when the build says succeeded" \
+  1 "these chroots did not" STUB_WATCH_HANGS=0 STUB_STATES="succeeded" \
+  STUB_CHROOTS="f43=succeeded,f44=failed,rawhide=succeeded"
+
+# All three green is the ordinary pass, and each is reported.
+run_case "all chroots succeeding passes and is reported" \
+  0 "rawhide: succeeded" STUB_WATCH_HANGS=0 STUB_STATES="succeeded" \
+  STUB_CHROOTS="f43=succeeded,f44=succeeded,rawhide=succeeded"
+
+# The build settling before its chroots do must not end the wait early.
+CASE_TIMEOUT=4 run_case "build succeeded but a chroot still running keeps waiting" \
+  0 "chroots still going: f44(running)" STUB_WATCH_HANGS=0 STUB_STATES="succeeded" \
+  STUB_CHROOTS="f43=succeeded,f44=running,rawhide=succeeded"
+
+# The step limit case: the build is still running when our wait budget expires.
+# This must NOT be reported as a failure — that is what turned every green
+# release red — and it must not try to fetch logs that do not exist yet.
+CASE_TIMEOUT=3 run_case "still-running build at budget expiry exits 0" \
+  0 "has not finished within" STUB_WATCH_HANGS=1 STUB_STATES="running"
+
+CASE_TIMEOUT=3 run_case "still-running build says so rather than claiming failure" \
+  0 "COPR will finish it" STUB_WATCH_HANGS=1 STUB_STATES="running"
 
 echo
 echo "passed: $PASS  failed: $FAIL"