From ce6769a89994325d71f816bbce38878bbcd768bd Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Mon, 17 Aug 2026 07:13:11 +0300 Subject: [PATCH 1/2] ci: write releases from one paced job, not from 107 at once Every matrix job uploaded its images to three shared releases with softprops/action-gh-release, so a nightly fired on the order of 400 concurrent asset writes at `nightly`, `latest` and the dated tag. OpenIPC/firmware ran exactly this design and the releases API answered HTTP 500 under the contention -- run 27108181857 built every board and still went red on 14 jobs, all of them in their upload step. Firmware then hit the sequel: consolidating into one job but keeping concurrent uploads tripped the per-actor secondary rate limit at 350 of 393 assets. This lands where that ended up, without the two failures in between. Matrix jobs now stage what they produced into dist/ and hand it to actions/upload-artifact. A single publish job downloads all of it and drives the release writes with gh: one asset at a time, paced ~1/s under the per-minute mutation ceiling, with exponential backoff so a transient 403/429 is retried rather than failing the nightly. The single-writer property is what fixes the 500s; the pacing is what keeps the fix from tripping the other limit. The dated release keeps the full asset set, because enrich_manifest.py only ever reads dated releases. `nightly` and `latest` are delivery aliases for flashers and now get images only: re-uploading the size sidecars to two more tags is pure rate-limit cost, and nothing reads them from those tags. Staging copies whichever of the two naming schemes the build used -- compound devices rename to -.tgz in the workspace root, single-underscore ones leave openipc.--.tgz under openipc/output/images/ -- so asset names are byte-identical to today. Checked both against enrich_manifest.py's COMPOUND_RE and SIMPLE_RE. ci-gate covers publish, so a failed release write fails the run instead of passing quietly. The staging loop uses `if` rather than a trailing `&&` chain: the step runs under `bash -e`, where a chain that tests false is only harmless because something after it happens to reset the status. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/master.yml | 189 +++++++++++++++++++++++++++-------- 1 file changed, 150 insertions(+), 39 deletions(-) diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index 05766240e..9e2179029 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -269,50 +269,44 @@ jobs: echo "SIZES=sizes.${NAME}.json" >> ${GITHUB_ENV} fi - - name: Upload firmware (dated) + # Stage everything this device produced into one flat directory, so the + # artifact has a predictable shape no matter which of the two naming + # schemes the build used (compound devices rename to -nor.tgz in + # the workspace root, simple ones leave openipc.-nor-.tgz + # under openipc/output/images/). + - name: Stage artifacts if: >- github.event_name != 'pull_request' && (env.NORFW || env.NANDFW) - uses: softprops/action-gh-release@v2 - with: - tag_name: ${{ needs.preflight.outputs.build_id }} - prerelease: true - body: | - sha=${{ needs.preflight.outputs.head_sha }} - short=${{ needs.preflight.outputs.short_sha }} - built_at=${{ needs.preflight.outputs.built_at }} - files: | - ${{env.NORFW}} - ${{env.NANDFW}} - ${{env.SIZES}} - - - name: Upload firmware (rolling nightly) - if: >- - github.event_name != 'pull_request' && - (env.NORFW || env.NANDFW) - uses: softprops/action-gh-release@v2 - with: - tag_name: nightly - body: | - sha=${{ needs.preflight.outputs.head_sha }} - short=${{ needs.preflight.outputs.short_sha }} - built_at=${{ needs.preflight.outputs.built_at }} - files: | - ${{env.NORFW}} - ${{env.NANDFW}} - ${{env.SIZES}} - - - name: Upload firmware (latest — legacy alias) + run: | + mkdir -p dist + for f in "${NORFW:-}" "${NANDFW:-}" "${SIZES:-}"; do + # An `if` rather than a `&&` chain: the step runs under `bash -e`, + # where a trailing `&&` that tests false is only harmless because + # something after it resets the status. + if [ -n "$f" ] && [ -e "$f" ]; then + cp -a "$f" dist/ + fi + done + ls -la dist + + # Hand the images to the single `publish` job instead of writing releases + # from here. 107 matrix jobs each deleting and re-uploading assets on the + # SAME three shared releases is ~400 concurrent writes; OpenIPC/firmware + # ran exactly that design and the releases API answered HTTP 500 under the + # contention (run 27108181857: every board built, 14 jobs red in their + # upload step). Consolidating every release write into one job removes the + # concurrent-writer contention that causes it. + - name: Upload build artifacts if: >- github.event_name != 'pull_request' && (env.NORFW || env.NANDFW) - uses: softprops/action-gh-release@v2 + uses: actions/upload-artifact@v4 with: - tag_name: latest - files: | - ${{env.NORFW}} - ${{env.NANDFW}} - ${{env.SIZES}} + name: fw-${{ matrix.platform }} + if-no-files-found: ignore + retention-days: 1 + path: dist/* - name: Send binary if: github.event_name != 'pull_request' && env.NORFW @@ -325,6 +319,117 @@ jobs: HTTP=$(curl -s -o /dev/null -w %{http_code} https://api.telegram.org/bot${TG_TOKEN}/sendDocument -F chat_id=${TG_CHANNEL} -F caption="${TG_HEADER}" -F document=@${NORFW}) echo Telegram response: ${HTTP} + # All release writes happen here, once, so only a SINGLE job ever touches the + # shared `nightly`/`latest` releases and the per-run dated one. Best-effort by + # design: publish whatever the matrix produced even if some devices failed, + # which matches enrich_manifest.py -- it indexes whatever assets exist. Never + # runs on a pull request; there are no artifacts to publish there. + publish: + name: Publish releases + needs: [preflight, select, buildroot] + if: >- + github.event_name != 'pull_request' && + needs.preflight.outputs.should_build == 'true' && + contains(fromJSON('["success", "failure"]'), needs.buildroot.result) + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - name: Download build artifacts + uses: actions/download-artifact@v4 + continue-on-error: true + with: + pattern: fw-* + path: dist + merge-multiple: true + + - name: Collect assets + id: collect + run: | + mkdir -p dist + count=$(find dist -type f | wc -l) + echo "Collected ${count} asset(s):" + ls -la dist || true + echo "count=${count}" >> "$GITHUB_OUTPUT" + + # Drive the release writes with gh rather than softprops/action-gh-release, + # which uploads assets CONCURRENTLY with no throttle or retry knob. Firing + # a few hundred asset writes at three releases trips GitHub's per-actor + # secondary rate limit; here every asset goes up one at a time, paced under + # the per-minute mutation ceiling, with exponential backoff so a transient + # 403/429 is retried rather than failing the nightly. + - name: Publish releases (paced, retry-aware) + if: steps.collect.outputs.count != '0' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + BUILD_ID: ${{ needs.preflight.outputs.build_id }} + HEAD_SHA: ${{ needs.preflight.outputs.head_sha }} + SHORT_SHA: ${{ needs.preflight.outputs.short_sha }} + BUILT_AT: ${{ needs.preflight.outputs.built_at }} + run: | + set -euo pipefail + + gh_retry() { + local n=0 max=6 delay=15 + until "$@"; do + n=$((n + 1)) + if [ "$n" -ge "$max" ]; then + echo "::error::gh failed after ${max} attempts: $*" + return 1 + fi + echo "::warning::gh attempt ${n} failed, sleeping ${delay}s before retry" + sleep "$delay" + delay=$((delay * 2)) + done + } + + ensure_release() { # [extra gh release create args...] + local tag="$1"; shift + if ! gh release view "$tag" >/dev/null 2>&1; then + gh_retry gh release create "$tag" --target "$HEAD_SHA" --notes "$NOTES" "$@" + fi + } + + # One asset at a time, paced ~1/s to stay under the per-minute + # content-mutation ceiling that triggers the secondary rate limit. + upload_paced() { # ... + local tag="$1"; shift + local total=$# i=0 + for f in "$@"; do + i=$((i + 1)) + echo "[${tag}] (${i}/${total}) ${f##*/}" + gh_retry gh release upload "$tag" "$f" --clobber + sleep 1 + done + } + + NOTES=$(printf 'sha=%s\nshort=%s\nbuilt_at=%s\n' "$HEAD_SHA" "$SHORT_SHA" "$BUILT_AT") + + # Full asset set (images + size sidecars) -> the dated release, which + # is the only one enrich_manifest.py reads. + mapfile -t DATED < <(find dist -maxdepth 1 -type f | sort) + # nightly/latest are firmware-delivery aliases for flashers, so ship + # only the images there. Re-uploading the size sidecars to two more + # releases is pure rate-limit cost: the manifest never reads them from + # these tags, and nothing else does either. + mapfile -t IMAGES < <(find dist -maxdepth 1 -type f -name '*.tgz' | sort) + + # --- dated: immutable per-build history --- + ensure_release "$BUILD_ID" --prerelease --title "$BUILD_ID" + upload_paced "$BUILD_ID" "${DATED[@]}" + + # --- rolling nightly: move tag + body to this build, refresh images --- + ensure_release nightly + gh_retry gh release edit nightly --notes "$NOTES" + gh_retry gh api -X PATCH "repos/${GH_REPO}/git/refs/tags/nightly" -f sha="$HEAD_SHA" -F force=true + upload_paced nightly "${IMAGES[@]}" + + # --- latest: legacy alias --- + ensure_release latest + gh_retry gh api -X PATCH "repos/${GH_REPO}/git/refs/tags/latest" -f sha="$HEAD_SHA" -F force=true + upload_paced latest "${IMAGES[@]}" + # Single umbrella status check covering the whole run, so branch protection # can require one context instead of a hardcoded "Build ()" per # device -- the matrix is now dynamic, and its job names change per PR. @@ -332,7 +437,7 @@ jobs: # change, because nothing built one. ci-gate: name: CI Gate - needs: [select, preflight, buildroot] + needs: [select, preflight, buildroot, publish] # always() so the gate still reports when a needed job fails -- but not on # a run the guard above deliberately skipped, where it would read # select=skipped and turn a saved build into a red run. @@ -344,7 +449,7 @@ jobs: steps: - name: Require selection + device matrix to succeed run: | - echo "select=${{ needs.select.result }} preflight=${{ needs.preflight.result }} buildroot=${{ needs.buildroot.result }}" + echo "select=${{ needs.select.result }} preflight=${{ needs.preflight.result }} buildroot=${{ needs.buildroot.result }} publish=${{ needs.publish.result }}" if [ "${{ needs.select.result }}" != "success" ]; then echo "::error::device selection did not succeed"; exit 1 fi @@ -367,3 +472,9 @@ jobs: fi echo "device matrix OK (no devices to build)" fi + # publish is skipped on PRs and on a run that built nothing; only a + # real failure of the single release-writer should fail the gate. + case "${{ needs.publish.result }}" in + success|skipped) echo "publish OK (${{ needs.publish.result }})" ;; + *) echo "::error::publish result=${{ needs.publish.result }}"; exit 1 ;; + esac From a6114ac52d4414f56cff5b2c0aee7f67e1b3b8fd Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Mon, 17 Aug 2026 07:35:05 +0300 Subject: [PATCH 2/2] ci: stop the publish job succeeding without publishing Two High findings on #121, both real. The download step carried continue-on-error: true, so a transient or real artifact-download failure produced an empty dist/, the publish step's count guard skipped it, the job went green, and ci-gate accepts publish=success. A nightly could silently write nothing while the run stayed green. The flag stays -- it exists so a matrix where every device failed, and so uploaded no fw-* artifact at all, does not hard-fail before the gate can report why -- but Collect now turns it back into a failure for every other case: if the matrix went green, a download that did not succeed is an error, and zero collected assets is an error. Only "matrix failed and produced nothing" is still tolerated, and the gate fails that run on the matrix result anyway. The second finding said count could read " 0" because wc pads. On GNU coreutils, which is what the runner has, it does not -- that is BSD behaviour, and `printf '' | wc -l` here gives an unpadded 0. Normalised anyway, since it costs a tr and stops the guard depending on which wc is in front of it. Chasing it did surface a worse hole it sits next to. dist/ can hold sidecars and no images -- one device that produced a size report and no .tgz is enough -- and count is then 1, so publish ran, force-moved the nightly and latest tag refs, and uploaded zero images. The two tags users flash from would point at a build with no firmware behind them. Both tags now move only when there is at least one image to move them to; otherwise they stay on yesterday's, with a warning. Checked all four states offline, since none of this path can run on a pull request: green matrix with a broken download, green matrix with an empty dist, failed matrix with nothing produced, and a normal night. The first two now fail and used to pass. Note the same continue-on-error and unnormalised wc are in OpenIPC/firmware's publish job, which is where this one came from. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/master.yml | 56 +++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index 9e2179029..956659057 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -335,7 +335,13 @@ jobs: permissions: contents: write steps: + # continue-on-error only so that a matrix where every device failed -- + # and therefore uploaded no fw-* artifact at all -- does not hard-fail + # here before the gate can report why. Collect below turns that back into + # a failure for every case except that one, so a download that breaks can + # no longer publish nothing and call it success. - name: Download build artifacts + id: download uses: actions/download-artifact@v4 continue-on-error: true with: @@ -347,11 +353,30 @@ jobs: id: collect run: | mkdir -p dist - count=$(find dist -type f | wc -l) + # tr because wc pads its output on some platforms (BSD does, the GNU + # coreutils on the runner does not) and a padded " 0" would read + # as non-empty to the `!= '0'` guard below. + count=$(find dist -type f | wc -l | tr -d '[:space:]') echo "Collected ${count} asset(s):" ls -la dist || true echo "count=${count}" >> "$GITHUB_OUTPUT" + # A matrix that went green must have produced artifacts. Zero assets + # after that means the download failed, not that there was nothing to + # publish -- and publishing nothing quietly is exactly how a nightly + # goes missing while the run stays green. + if [ "${{ needs.buildroot.result }}" = "success" ]; then + if [ "${{ steps.download.outcome }}" != "success" ]; then + echo "::error::artifact download failed after a fully successful matrix" + exit 1 + fi + if [ "${count}" -eq 0 ]; then + echo "::error::no artifacts collected although every device built" + exit 1 + fi + elif [ "${count}" -eq 0 ]; then + echo "::notice::matrix failed and produced nothing; no release to write" + # Drive the release writes with gh rather than softprops/action-gh-release, # which uploads assets CONCURRENTLY with no throttle or retry knob. Firing # a few hundred asset writes at three releases trips GitHub's per-actor @@ -419,16 +444,25 @@ jobs: ensure_release "$BUILD_ID" --prerelease --title "$BUILD_ID" upload_paced "$BUILD_ID" "${DATED[@]}" - # --- rolling nightly: move tag + body to this build, refresh images --- - ensure_release nightly - gh_retry gh release edit nightly --notes "$NOTES" - gh_retry gh api -X PATCH "repos/${GH_REPO}/git/refs/tags/nightly" -f sha="$HEAD_SHA" -F force=true - upload_paced nightly "${IMAGES[@]}" - - # --- latest: legacy alias --- - ensure_release latest - gh_retry gh api -X PATCH "repos/${GH_REPO}/git/refs/tags/latest" -f sha="$HEAD_SHA" -F force=true - upload_paced latest "${IMAGES[@]}" + # nightly and latest only ever move when there is firmware to move + # them to. dist/ can hold sidecars and no images -- one device that + # produced a size report and no .tgz is enough -- and pointing the two + # tags users flash from at a build with no images behind them is worse + # than leaving them on yesterday's. + if [ "${#IMAGES[@]}" -eq 0 ]; then + echo "::warning::no images in this build; leaving nightly and latest where they are" + else + # --- rolling nightly: move tag + body to this build, refresh images --- + ensure_release nightly + gh_retry gh release edit nightly --notes "$NOTES" + gh_retry gh api -X PATCH "repos/${GH_REPO}/git/refs/tags/nightly" -f sha="$HEAD_SHA" -F force=true + upload_paced nightly "${IMAGES[@]}" + + # --- latest: legacy alias --- + ensure_release latest + gh_retry gh api -X PATCH "repos/${GH_REPO}/git/refs/tags/latest" -f sha="$HEAD_SHA" -F force=true + upload_paced latest "${IMAGES[@]}" + fi # Single umbrella status check covering the whole run, so branch protection # can require one context instead of a hardcoded "Build ()" per