From bc546cdf88d565b4f71174acdaffc50c1d428d4e Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 23 Jun 2026 18:22:07 -0700 Subject: [PATCH 1/2] Mark artifact-cleanup step continue-on-error so it never reds the run The cleanup-artifacts step is best-effort, but set -euo pipefail + the per-command guards only cover expected list/delete failures; a truly unexpected failure (e.g. gh itself) would still fail the job and red the run. Add continue-on-error: true to the delete step in both publish-release.yml and test-pull-request.yml so the best-effort intent is enforced at the workflow level, and note it in the AGENTS.md recipe. Surfaced by Copilot on the NxWitness re-sync (ptr727/NxWitness#443). Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/publish-release.yml | 5 +++-- .github/workflows/test-pull-request.yml | 5 +++-- AGENTS.md | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 79194ff5..7ae7addd 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -164,11 +164,12 @@ jobs: actions: write steps: - name: Delete workflow artifacts step + # Best-effort housekeeping: continue-on-error guarantees a failure here (even an unexpected one) never reds the + # run; the in-script guards still log warnings and delete as many artifacts as possible. + continue-on-error: true env: GH_TOKEN: ${{ github.token }} run: | - # Best-effort housekeeping: a failed list or delete logs a warning and continues so it never reds the run, - # while `set -e` still catches unexpected failures. set -euo pipefail if ! ids=$(gh api repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/artifacts --paginate \ --jq '.artifacts[].id'); then diff --git a/.github/workflows/test-pull-request.yml b/.github/workflows/test-pull-request.yml index eb8211c5..94618d28 100644 --- a/.github/workflows/test-pull-request.yml +++ b/.github/workflows/test-pull-request.yml @@ -152,11 +152,12 @@ jobs: actions: write steps: - name: Delete workflow artifacts step + # Best-effort housekeeping: continue-on-error guarantees a failure here (even an unexpected one) never reds the + # run; the in-script guards still log warnings and delete as many artifacts as possible. + continue-on-error: true env: GH_TOKEN: ${{ github.token }} run: | - # Best-effort housekeeping: a failed list or delete logs a warning and continues so it never reds the run, - # while `set -e` still catches unexpected failures. set -euo pipefail if ! ids=$(gh api repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/artifacts --paginate \ --jq '.artifacts[].id'); then diff --git a/AGENTS.md b/AGENTS.md index 436bfa97..689e1c3a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -192,7 +192,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, and tolerant of individual list/delete failures (warn and continue, never red the run). 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. **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. - **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. - **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 212dab64be45620ffbea0763039f57c74cc054f1 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 23 Jun 2026 18:23:06 -0700 Subject: [PATCH 2/2] Tighten cleanup continue-on-error comment to one line Keep the intent note to a single concise line; don't grow comments. --- .github/workflows/publish-release.yml | 3 +-- .github/workflows/test-pull-request.yml | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 7ae7addd..b1656674 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -164,8 +164,7 @@ jobs: actions: write steps: - name: Delete workflow artifacts step - # Best-effort housekeeping: continue-on-error guarantees a failure here (even an unexpected one) never reds the - # run; the in-script guards still log warnings and delete as many artifacts as possible. + # 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 }} diff --git a/.github/workflows/test-pull-request.yml b/.github/workflows/test-pull-request.yml index 94618d28..45774261 100644 --- a/.github/workflows/test-pull-request.yml +++ b/.github/workflows/test-pull-request.yml @@ -152,8 +152,7 @@ jobs: actions: write steps: - name: Delete workflow artifacts step - # Best-effort housekeeping: continue-on-error guarantees a failure here (even an unexpected one) never reds the - # run; the in-script guards still log warnings and delete as many artifacts as possible. + # 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 }}