From 068cea09197a48d31c2deabe09358c21c2ea3a07 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 26 Jun 2026 10:51:27 -0700 Subject: [PATCH 1/2] Clean up transfer artifacts surgically at consumption, not blanket-delete Fixes #214. The terminal cleanup-artifacts job (duplicated in test-pull-request.yml and publish-release.yml) blanket-deleted every run artifact (.artifacts[].id). Added to control huge release-asset artifacts (KiCadLibrary) filling the storage quota, it overstepped: it also destroyed diagnostic/log artifacts needed to debug a failed run. - Delete each transfer artifact at its point of consumption, by exact name/pattern: the github-release job deletes release-asset--* after attaching them to the release; publish-pypi deletes pypilibrary-build- after publishing. Grant actions:write where deletion happens (the publish caller for the reusable github-release job; publish-pypi directly). - Remove the blanket cleanup-artifacts job from both workflows. retention-days:1 on every upload is the failure-path backstop (a job that dies before its consumer leaves its artifact, reaped within a day); a failed run is reported normally. Diagnostics and build-records are no longer destroyed. - Rewrite the AGENTS.md artifact-retention rule to the consume-then-delete pattern so downstream job customizations honor it. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/build-release-task.yml | 17 ++++++++ .github/workflows/publish-release.yml | 50 ++++++++++-------------- .github/workflows/test-pull-request.yml | 28 ------------- AGENTS.md | 2 +- 4 files changed, 38 insertions(+), 59 deletions(-) diff --git a/.github/workflows/build-release-task.yml b/.github/workflows/build-release-task.yml index 46935f9f..669eb3c6 100644 --- a/.github/workflows/build-release-task.yml +++ b/.github/workflows/build-release-task.yml @@ -208,3 +208,20 @@ jobs: LICENSE README.md ./Publish/* + + # Surgical cleanup at the point of consumption: the release-asset--* transfer artifacts now have durable + # copies on the release, so delete them by exact pattern to free the storage quota - scoped to this branch's + # assets, leaving diagnostics and any other artifacts. A job that fails before this leaves its artifacts for the + # retention-days: 1 backstop. Needs the caller to grant `actions: write` (publish-release's publish job does). + - name: Delete consumed release asset artifacts step + if: ${{ inputs.expect_release_assets }} + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + ids=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/${{ github.run_id }}/artifacts" --paginate \ + --jq ".artifacts[] | select(.name | startswith(\"release-asset-${{ inputs.branch }}-\")) | .id") + for id in $ids; do + gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$id" \ + || echo "::warning::Failed to delete artifact $id; retention-days backstop will reap it." + done diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index a2d154bb..5b4eb894 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -77,6 +77,8 @@ jobs: secrets: inherit permissions: contents: write + # actions:write lets the github-release job delete the release-asset-* artifacts it consumes (surgical cleanup). + actions: write with: ref: ${{ matrix.branch }} branch: ${{ matrix.branch }} @@ -100,12 +102,12 @@ jobs: environment: name: pypi url: https://pypi.org/project/ptr727-projecttemplate-library/ - # id-token:write for Trusted Publishing's OIDC exchange, contents:read for repo metadata, actions:read so - # download-artifact can fetch this run's build artifact. + # id-token:write for Trusted Publishing's OIDC exchange, contents:read for repo metadata, actions:write so + # download-artifact can fetch this run's build artifact and the surgical cleanup step can delete it afterwards. permissions: id-token: write contents: read - actions: read + actions: write steps: @@ -123,6 +125,21 @@ jobs: # Skip rather than fail when the version already exists; the weekly republish re-uploads unchanged versions. skip-existing: true + # Surgical cleanup at the point of consumption: the pypilibrary-build- artifact has been published, so + # delete it by exact name to free the storage quota. A failure before this leaves it for the retention-days: 1 + # backstop. + - name: Delete consumed PyPI build artifact step + env: + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + id=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/${{ github.run_id }}/artifacts" --paginate \ + --jq ".artifacts[] | select(.name == \"pypilibrary-build-${{ matrix.branch }}\") | .id") + if [[ -n "$id" ]]; then + gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$id" \ + || echo "::warning::Failed to delete artifact $id; retention-days backstop will reap it." + fi + # Caller-gated to main: the badge and Docker Hub overview have no per-branch context, so they update only when main # is among the published branches (a develop-only push skips them). One invocation, not a per-branch matrix leg. date-badge: @@ -144,30 +161,3 @@ jobs: contents: read with: ref: main - - # Release artifacts are an intra-run handoff (durable copies live on the GitHub release), so leaving them accumulates - # against the small account-wide storage quota; delete them once every consumer has read them. - cleanup-artifacts: - name: Delete workflow artifacts job - needs: [setup, publish, publish-pypi, date-badge, docker-readme] - if: ${{ always() && needs.setup.outputs.publish == 'true' }} - runs-on: ubuntu-latest - permissions: - actions: write - steps: - - name: Delete workflow artifacts step - # continue-on-error: best-effort housekeeping must never red the run, even on an unexpected failure. - continue-on-error: true - env: - GH_TOKEN: ${{ github.token }} - run: | - set -euo pipefail - if ! ids=$(gh api repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/artifacts --paginate \ - --jq '.artifacts[].id'); then - echo "::warning::Could not list run artifacts; skipping cleanup (storage may not be freed)." - ids="" - fi - for artifact_id in $ids; do - gh api --method DELETE "repos/${{ github.repository }}/actions/artifacts/$artifact_id" \ - || echo "::warning::Failed to delete artifact $artifact_id; continuing." - done diff --git a/.github/workflows/test-pull-request.yml b/.github/workflows/test-pull-request.yml index 838f90bc..7d0fcb74 100644 --- a/.github/workflows/test-pull-request.yml +++ b/.github/workflows/test-pull-request.yml @@ -139,31 +139,3 @@ jobs: # smoke-build may be legitimately skipped (no target changed); only failure/cancelled blocks. exit_on_result "unit-test" "${{ needs.unit-test.result }}" exit_on_result "smoke-build" "${{ needs.smoke-build.result }}" - - # Smoke builds gate every release-asset upload on `!smoke`, but actions like docker/build-push-action can still emit - # a build-record artifact, so this terminal cleanup deletes the run's artifacts to keep them off the storage quota. - # Independent of `check-workflow-status` so housekeeping never gates the required merge check. - cleanup-artifacts: - name: Delete workflow artifacts job - needs: [smoke-build] - if: always() - runs-on: ubuntu-latest - permissions: - actions: write - steps: - - name: Delete workflow artifacts step - # continue-on-error: best-effort housekeeping must never red the run, even on an unexpected failure. - continue-on-error: true - env: - GH_TOKEN: ${{ github.token }} - run: | - set -euo pipefail - if ! ids=$(gh api repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/artifacts --paginate \ - --jq '.artifacts[].id'); then - echo "::warning::Could not list run artifacts; skipping cleanup (storage may not be freed)." - ids="" - fi - for artifact_id in $ids; do - gh api --method DELETE "repos/${{ github.repository }}/actions/artifacts/$artifact_id" \ - || echo "::warning::Failed to delete artifact $artifact_id; continuing." - done diff --git a/AGENTS.md b/AGENTS.md index bdfb73d6..ead29b08 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -194,7 +194,7 @@ These conventions describe the target state. New and modified workflows must res - **Boolean inputs**: workflows triggered both via `workflow_call` and `workflow_dispatch` must declare each boolean input in *both* trigger blocks - one definition does not propagate to the other. `workflow_call` delivers booleans as actual booleans; `workflow_dispatch` delivers them as the *strings* `"true"`/`"false"`. Any `if:` consuming a boolean input must compare against both forms - `if: ${{ inputs.foo == true || inputs.foo == 'true' }}`. - **Reusable workflows**: job-level `permissions:` are validated *before* the `if:` evaluates, so even a skipped job needs valid permissions declared. A `release` job with `permissions: contents: write` and `if: ${{ inputs.publish }}` will still cause `startup_failure` on a caller that doesn't grant `contents: write`. Either declare permissions at the call site, or omit the inner block and inherit. - **Allowlist `success` and `skipped` explicitly** when chaining jobs across optional dependencies - `!= 'failure'` lets `cancelled` through (timeout, runner failure, manual cancel). Use `(needs.X.result == 'success' || needs.X.result == 'skipped')`. -- **Artifact retention**: workflow artifacts are an intra-run handoff only - durable copies live on the GitHub release, not in workflow artifacts - so they must not survive the run and accumulate against the small account-wide artifact-storage quota. **Every workflow that can produce artifacts ends with a terminal `cleanup-artifacts` job** that deletes the run's artifacts via the REST API: `permissions: actions: write`, `needs` the artifact producers, an `if:` that **includes** `always()` (so a failed run still cleans up) plus any workflow-specific gate (e.g. `publish-release.yml` adds `&& needs.setup.outputs.publish == 'true'` to run only on real publishes), independent of any required status check so housekeeping never gates a merge, `continue-on-error: true` on the delete step so even an unexpected failure never reds the run, and tolerant of individual list/delete failures (warn and continue). This covers not just `actions/upload-artifact` but build-records that actions emit automatically (e.g. `docker/build-push-action`'s `.dockerbuild`). Both `publish-release.yml` and `test-pull-request.yml` carry one; add one to any new artifact-producing entry workflow. Set `retention-days: 1` on explicit uploads as a backstop. +- **Artifact retention**: workflow artifacts are an intra-run handoff only - durable copies live on the GitHub release, not in workflow artifacts - so they must not survive the run and accumulate against the small account-wide artifact-storage quota. **Clean up each transfer artifact surgically at its point of consumption**: the job that downloads it deletes it by exact name/pattern right after consuming it (the `github-release` job deletes `release-asset--*` after attaching them to the release; `publish-release.yml`'s `publish-pypi` deletes `pypilibrary-build-` after publishing). Deletion needs `actions: write` granted on that job - for a reusable callee (e.g. `github-release` inside `build-release-task.yml`) the **caller** grants it (`publish-release.yml`'s `publish` job does). **Never blanket-delete the run's artifacts** (`gh api .../artifacts --jq '.artifacts[].id'`) - that also destroys diagnostic/log artifacts and the build-records actions emit automatically (`docker/build-push-action`'s `.dockerbuild`), which are exactly what you need to debug a failed run. Set `retention-days: 1` on **every** explicit `upload-artifact`: it is the failure-path backstop - a job that dies before its consumer runs leaves its artifact, reaped within a day - so no separate terminal cleanup job is needed. A derived repo customizing these jobs must preserve the consume-then-delete shape. - **Docker layer cache**: cache to/from a registry tag (`type=registry`, e.g. `buildcache-` on Docker Hub), not the GitHub Actions cache (`type=gha`), to keep large image layers off the 10 GB Actions cache. A **multi-image** repo uses a **per-image** buildcache tag (`:buildcache-` for each image, plus the base image's own tag and inline cache); it does not fall back to `type=gha` for the extra images. - **Tag pinning on releases**: when using `softprops/action-gh-release` (or any tag-creating action), pass `target_commitish` explicitly - without it, GitHub's REST API defaults the new tag to the repository's default branch instead of the commit that built the artifact. Pin it to the **exact built commit's SHA** (the publisher uses NBGV's `GitCommitId` output), not `github.sha` (wrong branch in the publisher's branch matrix - a `develop` leg runs with `github.sha` = main's tip) and not a branch name (a moving ref that a mid-run commit could advance past the built tree). From c19981db8fe2ac6eb37de2cf6d8dada890ffc57e Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 26 Jun 2026 11:07:08 -0700 Subject: [PATCH 2/2] Make consume-then-delete cleanup best-effort (Copilot review) A listing/delete API hiccup must not red a job whose release/publish already succeeded: tolerate a failed artifact-list (warn + skip) and add continue-on-error, and delete every matching artifact id rather than a single scalar (a rerun can upload duplicates). The retention-days: 1 backstop covers anything missed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/build-release-task.yml | 10 ++++++++-- .github/workflows/publish-release.yml | 14 ++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build-release-task.yml b/.github/workflows/build-release-task.yml index 669eb3c6..607951d5 100644 --- a/.github/workflows/build-release-task.yml +++ b/.github/workflows/build-release-task.yml @@ -215,12 +215,18 @@ jobs: # retention-days: 1 backstop. Needs the caller to grant `actions: write` (publish-release's publish job does). - name: Delete consumed release asset artifacts step if: ${{ inputs.expect_release_assets }} + # Best-effort: the release is already published, so a listing/delete hiccup must never red the job; the + # retention-days: 1 backstop reaps anything missed. Deletes every matching id (a rerun can upload duplicates). + continue-on-error: true env: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail - ids=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/${{ github.run_id }}/artifacts" --paginate \ - --jq ".artifacts[] | select(.name | startswith(\"release-asset-${{ inputs.branch }}-\")) | .id") + if ! ids=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/${{ github.run_id }}/artifacts" --paginate \ + --jq ".artifacts[] | select(.name | startswith(\"release-asset-${{ inputs.branch }}-\")) | .id"); then + echo "::warning::Could not list run artifacts; retention-days backstop will reap them." + ids="" + fi for id in $ids; do gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$id" \ || echo "::warning::Failed to delete artifact $id; retention-days backstop will reap it." diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 5b4eb894..272070d5 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -129,16 +129,22 @@ jobs: # delete it by exact name to free the storage quota. A failure before this leaves it for the retention-days: 1 # backstop. - name: Delete consumed PyPI build artifact step + # Best-effort: PyPI is already published, so a listing/delete hiccup must never red the job; the + # retention-days: 1 backstop reaps anything missed. Deletes every matching id (a rerun can upload duplicates). + continue-on-error: true env: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail - id=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/${{ github.run_id }}/artifacts" --paginate \ - --jq ".artifacts[] | select(.name == \"pypilibrary-build-${{ matrix.branch }}\") | .id") - if [[ -n "$id" ]]; then + if ! ids=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/${{ github.run_id }}/artifacts" --paginate \ + --jq ".artifacts[] | select(.name == \"pypilibrary-build-${{ matrix.branch }}\") | .id"); then + echo "::warning::Could not list run artifacts; retention-days backstop will reap them." + ids="" + fi + for id in $ids; do gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$id" \ || echo "::warning::Failed to delete artifact $id; retention-days backstop will reap it." - fi + done # Caller-gated to main: the badge and Docker Hub overview have no per-branch context, so they update only when main # is among the published branches (a develop-only push skips them). One invocation, not a per-branch matrix leg.