ci(v2.3.9): open "Crucible" — what the gates actually cover

.github/actionlint.yaml

@@ -19,3 +19,27 @@ self-hosted-runner:
     # session, which is why that workflow cannot run on a hosted runner. See
     # `Local_Only-Projects/antigravity-pr-review/README.md` in the workspace.
     - agy
+
+# v2.3.9 — one suppression, scoped to one file and one message.
+#
+# `pgo.yml` carries two steps deliberately disabled with `if: false`, each with a
+# written rationale above it: the BOLT bench and determinism gates were switched
+# off because the harness benches the wrong binary, and "a gate that cannot
+# measure its subject is worse than no gate". The steps are kept rather than
+# deleted so the shape survives for whoever re-enables them behind a harness that
+# benches the BOLT-optimized binary itself.
+#
+# actionlint's `if-cond` rule flags a constant condition, which is correct in
+# general and wrong about intent here.
+#
+# THE COST, stated rather than buried: this silences the rule for the whole file,
+# so a genuinely accidental `if: false` added to `pgo.yml` later would not be
+# caught. actionlint has no line-scoped ignore, and the alternatives are worse —
+# deleting the steps loses the documented shape, and rewriting the condition to
+# something non-constant hides the intent from the reader to satisfy a linter.
+# Every other workflow keeps the rule; verified by adding an `if: false` to
+# `security.yml` and confirming it still fails.
+paths:
+  .github/workflows/pgo.yml:
+    ignore:
+      - 'constant expression "false" in condition'

.github/scripts/apt-install-retry.sh

@@ -0,0 +1,61 @@
+#!/usr/bin/env bash
+# Install one apt package, bounded and retried.
+#
+# v2.3.9 A5b. The cross-compile gate provisions glibc headers for bindgen with a
+# bare `apt-get update && apt-get install`. Both are network fetches with no
+# timeout of their own, so when a mirror stalls the step hangs until the JOB
+# timeout fires — 25 minutes for `libretro-cross` — and the run is reported as
+# cancelled rather than as what it was.
+#
+# That is not hypothetical. During the v2.3.7 cut this hung four separate times
+# across two PRs, always in a setup or provisioning step and never in a compile
+# or test step: twice in `rust-setup`, once in the armhf provision, once in the
+# aarch64 provision. Each cost 25-45 minutes and needed a manual re-run. The
+# per-job `timeout-minutes` added in #400 bounded the damage correctly; nothing
+# addressed the fragility underneath it.
+#
+# Two bounds, doing different jobs:
+#
+#   * `timeout` per command, so a stalled fetch fails in minutes rather than
+#     consuming the job's entire budget. The job timeout is a backstop against a
+#     hang; this is the thing that actually notices one.
+#   * Three attempts with linear backoff, because the observed failure is
+#     transient — a re-run has cleared it every time.
+#
+# Deliberately NOT a general-purpose apt wrapper: one package, from a workflow
+# `env:` (never from event data, which is the injection vector the Actions
+# security guidance warns about), and a hard failure if it is unset.
+set -euo pipefail
+
+if [ -z "${APT_PACKAGE:-}" ]; then
+    echo "::error::APT_PACKAGE is unset; refusing to guess what to install" >&2
+    exit 1
+fi
+
+# Bounds chosen from observed behaviour, not from taste: a healthy `update` on
+# these runners is a few seconds and a healthy `install` well under a minute, so
+# these are roughly an order of magnitude of headroom. Long enough that a merely
+# slow mirror still succeeds; short enough that three full attempts fit inside
+# the 25-minute job budget with room for the build that follows.
+readonly UPDATE_TIMEOUT=180
+readonly INSTALL_TIMEOUT=300
+readonly ATTEMPTS=3
+
+for attempt in $(seq 1 "$ATTEMPTS"); do
+    if timeout "$UPDATE_TIMEOUT" sudo apt-get update -qq &&
+        timeout "$INSTALL_TIMEOUT" sudo apt-get install -yq "$APT_PACKAGE"; then
+        echo "Installed ${APT_PACKAGE} on attempt ${attempt}."
+        exit 0
+    fi
+    # Reported per attempt rather than only on final failure: a run that
+    # succeeded on attempt 3 looks identical to one that succeeded on attempt 1
+    # in the job's conclusion, and the difference is the early warning that the
+    # mirrors are degrading.
+    echo "::warning::apt attempt ${attempt}/${ATTEMPTS} for ${APT_PACKAGE} failed or timed out"
+    if [ "$attempt" -lt "$ATTEMPTS" ]; then
+        sleep $((attempt * 15))
+    fi
+done
+
+echo "::error::Could not install ${APT_PACKAGE} after ${ATTEMPTS} attempts" >&2
+exit 1

.github/workflows/ci.yml

@@ -106,6 +106,7 @@ jobs:
       pull-requests: read
     outputs:
       code: ${{ steps.filter.outputs.code }}
+      accuracy: ${{ steps.filter.outputs.accuracy }}
     steps:
       # Full history: on `push`, dorny/paths-filter diffs against the before-SHA
       # using local git, so the default shallow clone (fetch-depth: 1) can miss
@@ -138,6 +139,34 @@ jobs:
               - '!NOTICE'
               - '!.gitignore'
               - '!.codegraph/**'
+            # v2.3.9 A5 — paths that can move an accuracy or visual vector.
+            #
+            # `test-roms` used to be FULL-run only, so a regular feature PR never
+            # ran the accuracy battery and a regression could not be caught on
+            # the PR that caused it: it landed, turned `main` red, and was fixed
+            # by a second PR. #396 (a PPU fix that legitimately moved
+            # `visual_regression__scanline_frame_180`) and #403 (the vector
+            # update that followed) are the worked example.
+            #
+            # Scoped by path rather than by event, because a vector can only move
+            # if something that PRODUCES one changed. Measured against the last
+            # 40 merged PRs: 11 touch these paths and 29 do not, so ~72% of PRs
+            # still pay nothing and the cost model the full-run flag exists for
+            # is preserved.
+            #
+            # `rustynes-gamedb` is in the list for a non-obvious reason: the
+            # per-game database rewrites the iNES header on load, so it changes
+            # what the emulator IS before a single cycle runs. That is how the
+            # v2.3.4 Sachen defect reached users.
+            accuracy:
+              - 'crates/rustynes-cpu/**'
+              - 'crates/rustynes-ppu/**'
+              - 'crates/rustynes-apu/**'
+              - 'crates/rustynes-mappers/**'
+              - 'crates/rustynes-core/**'
+              - 'crates/rustynes-gamedb/**'
+              - 'crates/rustynes-test-harness/**'
+              - 'tests/**'
 
   # fmt + clippy + rustdoc share one runner + one compile of the workspace's
   # dependency graph (clippy and rustdoc differ only in the final pass), so
@@ -330,14 +359,30 @@ jobs:
   test-roms:
     name: test (test-roms feature)
     # The SLOWEST job (release-mode compile + the heavy CPU / AccuracyCoin ROM
-    # batteries, ~20 min). FULL-run only: it is skipped on a regular feature PR
-    # and runs on push-to-`main` (every merge is accuracy-validated), the merge
-    # queue, dispatch, the weekly cron, and `release/*` PRs (so a release is
-    # proven before it is cut). This is the single biggest per-PR time/cost
-    # saving. Also gated on the fast lint job — don't pay the release compile
-    # when fmt/clippy already failed.
+    # batteries, ~20 min). Runs on a FULL run — push-to-`main` (every merge is
+    # accuracy-validated), the merge queue, dispatch, the weekly cron, and
+    # `release/*` PRs so a release is proven before it is cut — OR on any PR that
+    # touches a path able to move an accuracy or visual vector (the `accuracy`
+    # filter in the `changes` job, which carries the rationale).
+    #
+    # v2.3.9 A5. This was FULL-run only, which meant a regular feature PR never
+    # ran the battery, so an accuracy regression could not be caught on the PR
+    # that caused it — it landed, turned `main` red, and needed a second PR.
+    # Sharpening the point: `main`'s ruleset requires exactly ONE status context,
+    # `CI success`, and a SKIPPED job does not fail the aggregate. So the single
+    # gate on merging was reporting a pass for a property it had not tested.
+    #
+    # The per-PR saving the full-run flag exists for is preserved rather than
+    # traded away: measured over the last 40 merged PRs, 11 touch accuracy paths
+    # and 29 do not, so roughly 72% of PRs still skip this entirely.
+    #
+    # Also gated on the fast lint job — don't pay the release compile when
+    # fmt/clippy already failed.
     needs: [changes, lint, setup]
-    if: ${{ needs.changes.outputs.code == 'true' && needs.setup.outputs.full == 'true' }}
+    if: >-
+      ${{ needs.changes.outputs.code == 'true'
+      && (needs.setup.outputs.full == 'true'
+      || needs.changes.outputs.accuracy == 'true') }}
     runs-on: ubuntu-latest
     # Typical: ~29 min. Bounded so a hung job cannot hold the
     # concurrency group -- see the block comment above `concurrency`.
@@ -522,21 +567,27 @@ jobs:
       # regardless of how the sysroot lays out `usr/include`. The cross linker in
       # that package is unused — this gate is `cargo check` only.
       - name: Provision the aarch64 glibc headers for bindgen
+        if: matrix.target == 'aarch64-unknown-linux-gnu'
+        env:
+          APT_PACKAGE: gcc-aarch64-linux-gnu
+        run: .github/scripts/apt-install-retry.sh
+      - name: Export the aarch64 bindgen sysroot
         if: matrix.target == 'aarch64-unknown-linux-gnu'
         run: |
-          sudo apt-get update -qq
-          sudo apt-get install -yq gcc-aarch64-linux-gnu
           echo "BINDGEN_EXTRA_CLANG_ARGS_aarch64_unknown_linux_gnu=--sysroot=/usr/aarch64-linux-gnu -isystem /usr/aarch64-linux-gnu/include" >> "$GITHUB_ENV"
       # Provision the armhf glibc headers for bindgen, exactly as for aarch64
       # above. `gcc-arm-linux-gnueabihf` pulls `libc6-dev-armhf-cross`, landing
       # the 32-bit ARM glibc headers under `/usr/arm-linux-gnueabihf/include`.
       # The cross linker in that package is unused here — this gate is
       # `cargo check` only; the buildbot remains the authority on linking.
       - name: Provision the armhf glibc headers for bindgen
+        if: matrix.target == 'armv7-unknown-linux-gnueabihf'
+        env:
+          APT_PACKAGE: gcc-arm-linux-gnueabihf
+        run: .github/scripts/apt-install-retry.sh
+      - name: Export the armhf bindgen sysroot
         if: matrix.target == 'armv7-unknown-linux-gnueabihf'
         run: |
-          sudo apt-get update -qq
-          sudo apt-get install -yq gcc-arm-linux-gnueabihf
           echo "BINDGEN_EXTRA_CLANG_ARGS_armv7_unknown_linux_gnueabihf=--sysroot=/usr/arm-linux-gnueabihf -isystem /usr/arm-linux-gnueabihf/include" >> "$GITHUB_ENV"
       # Deliberately NOT the composite action's `targets:` input. That routes
       # through `dtolnay/rust-toolchain`, which installs the target for the

.github/workflows/release-auto.yml

@@ -62,7 +62,19 @@ jobs:
     runs-on: ubuntu-latest
     # Bounded like every other job (v2.3.7). `build` below cannot carry one —
     # `timeout-minutes` is not valid on a job that uses `uses:` — so its budget
-    # lives on the jobs inside `release.yml`.
+    # lives on the jobs inside `release.yml`, which already carry their own.
+    #
+    # Challenged in review on #406, which claimed the restriction was lifted in
+    # late 2022. It was not. GitHub's workflow-syntax and reuse-workflows pages
+    # state neither way, so it was checked against the schema rather than
+    # recalled; `actionlint` on exactly this shape:
+    #
+    #   when a reusable workflow is called with "uses", "timeout-minutes" is not
+    #   available. only following keys are allowed: "name", "uses", "with",
+    #   "secrets", "needs", "if", and "permissions"
+    #
+    # So adding one here is a hard syntax error, not the harmless no-op it would
+    # be if the key were merely ignored.
     timeout-minutes: 15
     outputs:
       should_release: ${{ steps.decide.outputs.should_release }}

.pre-commit-config.yaml

@@ -71,6 +71,24 @@ repos:
       - id: markdownlint
         args: [--config, .markdownlint.json]
 
+  # GitHub Actions workflow linting (v2.3.9).
+  #
+  # Pinned to the version installed here, matching how every other hook in this
+  # file is pinned: an unpinned linter that gains a rule turns a green tree red
+  # on someone else's machine, which is the trap `markdownlint` already documents
+  # (the local binary reports rules the pinned v0.39.0 does not).
+  #
+  # Added because a review disagreement on #406 could not be settled from
+  # GitHub's own documentation — whether `timeout-minutes` is valid on a job that
+  # calls a reusable workflow. `actionlint` encodes the job schema and answered it
+  # in one command. Seven workflow files were edited during the v2.3.7 cut with no
+  # schema check at all; `check-yaml` proves a file is YAML, not that it is a
+  # workflow.
+  - repo: https://github.com/rhysd/actionlint
+    rev: v1.7.12
+    hooks:
+      - id: actionlint
+
   # Rust formatting and linting (when code exists)
   - repo: local
     hooks: