From c2b8ae48ed524cf204cb02cd6e5cda501ca63881 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 3 Sep 2026 23:21:47 +0100 Subject: [PATCH 01/10] Install CI tools from releases and settle cache ownership The Whitaker step compiled `whitaker-installer` from crates.io whenever `cargo binstall` was missing or had no prebuilt artefact for the pinned version. Replace it with the shared `install-whitaker` action, which downloads the pinned prebuilt release and verifies it against a digest pinned inside the action, and let that action own the installer cache. Three steps claimed `~/.cargo/registry` and `~/.cargo/git`: the shared `setup-rust` action, `Swatinem/rust-cache`, and the shared `generate-coverage` action. Remove `Swatinem/rust-cache`, which also archived a `target` tree that a compiler cache should own, and pass `cache-provider: external` to `generate-coverage` so `setup-rust` is the single owner. Add the missing owner for the repository-local uv download and tool directories that `make spelling` populates. Nothing sets `RUSTC_WRAPPER`, so the `sccache` install downloaded a binary that served no compilation and had no cache owner; turn it off. Adopting a compiler cache is a separate, measured change. Both Ubicloud jobs now declare `timeout-minutes` so a hung step cannot bill to the platform's six-hour default, and every `leynos/shared-actions` reference pins 7d46a399558914f5a05074e55a560fec0269fd0d. The separate uninstrumented `cargo test` step repeated the suite the coverage run already executes, for a second full compile and no extra evidence. Drop it and give the instrumented run `all-features`, `all-targets`, and `doctests`; `all-features` names exactly the feature set the explicit list named. No job changes its runner label or shape. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- .github/workflows/ci.yml | 95 +++++++++++----------- .github/workflows/coverage-main.yml | 24 ++++-- .github/workflows/dependabot-automerge.yml | 2 +- 3 files changed, 66 insertions(+), 55 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ce195dc..bd5f153 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,12 +7,14 @@ on: jobs: build-test: runs-on: ubicloud-standard-8 + # Ubicloud jobs bill by the minute, so a hung step must not run to the + # 6-hour platform default. The 2026-09-03 baseline median was 1656 s. + timeout-minutes: 90 permissions: contents: read env: CARGO_TERM_COLOR: always BUILD_PROFILE: debug - WHITAKER_INSTALLER_VERSION: '0.2.7' # Bevy's render features make the coverage build heavy; lift the # shared-action cargo wall-clock cap (default 600 s) accordingly. RUN_RUST_CARGO_WAIT_TIMEOUT: '1800' @@ -24,67 +26,64 @@ jobs: # the pull request's merge base. fetch-depth: 0 - name: Setup Rust - uses: leynos/shared-actions/.github/actions/setup-rust@794e4801babcf68065c660fdf4781ad62be5d061 + uses: leynos/shared-actions/.github/actions/setup-rust@7d46a399558914f5a05074e55a560fec0269fd0d + with: + # Sole owner of ~/.cargo/registry and ~/.cargo/git for this job. It + # runs before the first cargo invocation so the lint step reads a + # warm registry. + cache-provider: github + # No RUSTC_WRAPPER is configured anywhere in this repository, so an + # sccache install would download a binary that never serves a + # compilation and would have no cache owner. Adopting sccache is a + # separate, measured change. + use-sccache: 'false' + - name: Cache uv tool layers + # `make spelling` drives uv with repository-local UV_CACHE_DIR and + # UV_TOOL_DIR, so these two directories are the whole uv surface. This + # job is the only one in the repository that installs uv tools, so it + # is necessarily both the reader and the single writer; there is no + # trunk job to designate as the writer instead. + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: | + .uv-cache + .uv-tools + key: uv-tools-v1-${{ runner.os }}-${{ runner.arch }}-${{ runner.environment }}-${{ hashFiles('Makefile', 'scripts/*.py') }} + restore-keys: | + uv-tools-v1-${{ runner.os }}-${{ runner.arch }}-${{ runner.environment }}- - name: Spelling run: make spelling - - name: Cache Rust dependencies - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - name: Check format run: make check-fmt - - name: Cache Whitaker installer - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 - with: - path: | - ~/.cargo/bin/whitaker-installer - ~/.cache/cargo-binstall - key: whitaker-installer-${{ runner.os }}-${{ runner.arch }}-${{ env.WHITAKER_INSTALLER_VERSION }} - name: Install the Whitaker Dylint suite - run: | - install_whitaker() { - # Prefer cargo-binstall, but fall back to a locked cargo install - # when binstall is unavailable OR its install attempt fails (e.g. - # no prebuilt binary for this version/target). Running the install - # inside the `if` condition keeps a binstall failure from aborting - # the step under `set -e`. - if cargo binstall --version >/dev/null 2>&1 \ - && cargo binstall --no-confirm --locked \ - "whitaker-installer@${WHITAKER_INSTALLER_VERSION}"; then - return 0 - fi - echo "cargo binstall unavailable or failed; building whitaker-installer from crates.io" - cargo install --locked whitaker-installer --version "${WHITAKER_INSTALLER_VERSION}" - } - # Reuse a cached installer only when its reported version is exactly - # the pinned version. A bare `command -v` check would keep a stale - # binary, and a substring/word match could accept a near-miss version. - installed_version="" - if command -v whitaker-installer >/dev/null 2>&1; then - installed_version="$(whitaker-installer --version 2>/dev/null | awk 'NF{print $NF}')" \ - || installed_version="" - fi - if [ "${installed_version}" = "${WHITAKER_INSTALLER_VERSION}" ]; then - echo "whitaker-installer ${WHITAKER_INSTALLER_VERSION} already present; skipping install" - else - install_whitaker - fi - whitaker-installer + # Downloads the pinned prebuilt installer and verifies it against a + # digest pinned in the action. The action owns the cache for the + # installer binary, its version marker, and ~/.local/share/whitaker. + uses: leynos/shared-actions/.github/actions/install-whitaker@7d46a399558914f5a05074e55a560fec0269fd0d + with: + installer-version: '0.2.7' + cache-provider: github - name: Lint run: | cargo clippy --all-targets --all-features -- -D warnings RUSTFLAGS="-D warnings" whitaker --all -- --all-targets --all-features - - name: Test - run: cargo test - # Coverage is generated with the shared action (replacing the bespoke - # cargo-llvm-cov steps) so the whole estate shares one recipe. The - # ratchet compares against the baseline written by coverage-main.yml. + # The instrumented run is this workflow's only test execution on Linux: + # a separate uninstrumented `cargo test` repeated the suite for no extra + # evidence and doubled the billed compile. `all-features` names exactly + # the set the explicit feature list used to name. - name: Generate coverage - uses: leynos/shared-actions/.github/actions/generate-coverage@794e4801babcf68065c660fdf4781ad62be5d061 + uses: leynos/shared-actions/.github/actions/generate-coverage@7d46a399558914f5a05074e55a560fec0269fd0d with: - features: render map text test-support observers-v1-spike + all-features: 'true' + all-targets: 'true' + doctests: 'true' output-path: lcov.info format: lcov use-cargo-nextest: 'false' with-ratchet: 'true' + # `setup-rust` above already owns ~/.cargo/registry and ~/.cargo/git; + # this keeps the action from becoming a second owner of them. + cache-provider: external # The CodeScene changed-line gate: diffs the PR against its merge # base and evaluates changed-line coverage. Guarded so secret-less # runs (forks, repos not yet onboarded) skip rather than fail. @@ -93,7 +92,7 @@ jobs: env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} if: env.CS_ACCESS_TOKEN != '' && github.event_name == 'pull_request' - uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@794e4801babcf68065c660fdf4781ad62be5d061 + uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@7d46a399558914f5a05074e55a560fec0269fd0d with: format: lcov mode: check diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index cb67fa5..9ff87f0 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -15,6 +15,9 @@ on: jobs: coverage-upload: runs-on: ubicloud-standard-8 + # Ubicloud jobs bill by the minute, so a hung step must not run to the + # 6-hour platform default. The 2026-09-03 baseline median was 561 s. + timeout-minutes: 60 permissions: contents: read env: @@ -26,22 +29,31 @@ jobs: steps: - uses: actions/checkout@v7 - name: Setup Rust - uses: leynos/shared-actions/.github/actions/setup-rust@794e4801babcf68065c660fdf4781ad62be5d061 - - name: Cache Rust dependencies - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + uses: leynos/shared-actions/.github/actions/setup-rust@7d46a399558914f5a05074e55a560fec0269fd0d + with: + # Sole owner of ~/.cargo/registry and ~/.cargo/git for this job, and + # the trunk writer whose entry every pull-request run restores. + cache-provider: github + # See ci.yml: nothing sets RUSTC_WRAPPER, so the sccache install + # would download a binary that serves no compilation. + use-sccache: 'false' - name: Generate coverage - uses: leynos/shared-actions/.github/actions/generate-coverage@794e4801babcf68065c660fdf4781ad62be5d061 + uses: leynos/shared-actions/.github/actions/generate-coverage@7d46a399558914f5a05074e55a560fec0269fd0d with: - features: render map text test-support observers-v1-spike + all-features: 'true' + all-targets: 'true' + doctests: 'true' output-path: lcov.info format: lcov use-cargo-nextest: 'false' with-ratchet: 'true' + # `setup-rust` above already owns ~/.cargo/registry and ~/.cargo/git. + cache-provider: external - name: Upload coverage data to CodeScene env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} if: env.CS_ACCESS_TOKEN != '' - uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@794e4801babcf68065c660fdf4781ad62be5d061 + uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@7d46a399558914f5a05074e55a560fec0269fd0d with: format: lcov access-token: ${{ env.CS_ACCESS_TOKEN }} diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml index ae8660b..9f85c06 100644 --- a/.github/workflows/dependabot-automerge.yml +++ b/.github/workflows/dependabot-automerge.yml @@ -28,6 +28,6 @@ jobs: # The token is not used for any external cloud auth. id-token: write if: ${{ github.event_name == 'workflow_dispatch' || github.actor == 'dependabot[bot]' }} - uses: leynos/shared-actions/.github/workflows/dependabot-automerge.yml@794e4801babcf68065c660fdf4781ad62be5d061 + uses: leynos/shared-actions/.github/workflows/dependabot-automerge.yml@7d46a399558914f5a05074e55a560fec0269fd0d with: pull-request-number: ${{ inputs.pull-request-number || github.event.pull_request.number }} From 6c78873f108bbef1ac8f65a6bfe8c3458ca14270 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 3 Sep 2026 23:22:02 +0100 Subject: [PATCH 02/10] Assert the CI rules as workflow contracts The placement, tool-install, and cache-ownership rules were previously enforced only by review. A reviewer had to notice, on every workflow edit, that a new cache step did not overlap an existing one or that an installer still preceded its first use. Encode the rules as tests so the change that breaks one fails, rather than the CI run that suffers from it. The contracts parse the workflow files into a small model instead of matching raw text, so a reordered key or a reflowed block scalar cannot defeat a rule. Cache ownership is modelled for the shared composite actions too: each one contributes the paths it caches unless the caller has taken them with `cache-provider: external`, which is what makes a duplicate owner of the Cargo registry visible. Six of the twelve contracts fail against the workflows as they stood before the preceding commit, so each rule is load bearing rather than decorative. `serde_norway` is a maintained fork of the unmaintained `serde_yaml`; it is a development dependency only. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- Cargo.toml | 4 + tests/support/workflow_cache_owners.rs | 144 +++++++++++++++ tests/support/workflow_model.rs | 230 +++++++++++++++++++++++ tests/workflow_contracts.rs | 243 +++++++++++++++++++++++++ 4 files changed, 621 insertions(+) create mode 100644 tests/support/workflow_cache_owners.rs create mode 100644 tests/support/workflow_model.rs create mode 100644 tests/workflow_contracts.rs diff --git a/Cargo.toml b/Cargo.toml index e87b48a..dc81e7d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -116,6 +116,10 @@ mockall = "0.13.1" static_assertions = "^1" tokio = { version = "1", features = ["macros", "rt-multi-thread"] } rspec = "1.0" +# Parses the workflow estate for the structural CI contracts in +# tests/workflow_contracts.rs. A maintained fork of the unmaintained +# serde_yaml, so serde derives and Value keep working. +serde_norway = "0.9" test_utils = { path = "test_utils" } trybuild = "1.0" # Non-optional in dev builds so the `trybuild` compile-pass fixture, which is a diff --git a/tests/support/workflow_cache_owners.rs b/tests/support/workflow_cache_owners.rs new file mode 100644 index 0000000..4801f7f --- /dev/null +++ b/tests/support/workflow_cache_owners.rs @@ -0,0 +1,144 @@ +//! Cache-ownership model for the repository's workflow jobs. +//! +//! Every mutable path a job caches must have exactly one owner. Some owners +//! are `actions/cache` steps in this repository; others are shared composite +//! actions that cache on the caller's behalf unless told that the caller owns +//! the path. This module reduces both kinds to the same `(path, owner)` list +//! so one contract can compare them. +//! +//! # Examples +//! +//! ```no_run +//! let owners = workflow_cache_owners::owners_for(&job); +//! assert!(owners.iter().all(|owner| !owner.path.is_empty())); +//! ``` + +use std::collections::BTreeMap; + +use crate::workflow_model::{Job, Step}; + +/// A single claim that one step caches one path. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CacheOwner { + /// Cached path, as written in the workflow or the shared action. + pub path: String, + /// Step that claims the path, named for a readable assertion message. + pub owner: String, +} + +/// Paths a shared composite action caches when `cache-provider` is `github`. +/// +/// These mirror the action definitions at +/// `leynos/shared-actions@7d46a399558914f5a05074e55a560fec0269fd0d`. A caller +/// that sets `cache-provider: external` takes the path away from the action, +/// which is how a second owner of the Cargo registry is avoided. +const SHARED_ACTION_CACHES: [(&str, &[&str]); 3] = [ + ("setup-rust", &["~/.cargo/registry", "~/.cargo/git"]), + ( + "generate-coverage", + &[ + "~/.cargo/bin/cargo-binstall", + "~/.cargo/bin/cargo-llvm-cov", + "~/.cargo/bin/cargo-nextest", + "~/.cargo/registry", + "~/.cargo/git", + ], + ), + ( + "install-whitaker", + &[ + "~/.cargo/bin/whitaker-installer", + "~/.cargo/bin/.whitaker-installer-version", + "~/.local/share/whitaker", + ], + ), +]; + +fn shared_action_name(uses: &str) -> Option<&str> { + let path = uses.split('@').next()?; + let name = path.strip_prefix("leynos/shared-actions/.github/actions/")?; + Some(name) +} + +fn is_cache_action(uses: &str) -> bool { + uses.split('@') + .next() + .is_some_and(|path| path == "actions/cache" || path.starts_with("actions/cache/")) +} + +fn step_label(step: &Step) -> String { + if step.name.is_empty() { + step.uses.clone() + } else { + step.name.clone() + } +} + +fn direct_owners(step: &Step) -> Vec { + if !is_cache_action(&step.uses) { + return Vec::new(); + } + let owner = step_label(step); + step.cache_paths() + .into_iter() + .map(|path| CacheOwner { + path, + owner: owner.clone(), + }) + .collect() +} + +fn shared_owners(step: &Step) -> Vec { + let Some(name) = shared_action_name(&step.uses) else { + return Vec::new(); + }; + // An empty input means the action's default, which is `github` for every + // shared action this repository calls. + let provider = step.input("cache-provider"); + if !provider.is_empty() && provider != "github" { + return Vec::new(); + } + let owner = step_label(step); + SHARED_ACTION_CACHES + .iter() + .filter(|(action, _)| *action == name) + .flat_map(|(_, paths)| paths.iter()) + .map(|path| CacheOwner { + path: (*path).to_owned(), + owner: owner.clone(), + }) + .collect() +} + +/// Returns every cache claim made by a job, in step order. +/// +/// `actions/cache/restore` and `actions/cache/save` halves of one split cache +/// share a step name prefix in practice; they are reported separately and the +/// caller decides whether the pair is a duplicate. +#[must_use] +pub fn owners_for(job: &Job) -> Vec { + job.steps + .iter() + .flat_map(|step| { + let mut claims = direct_owners(step); + claims.extend(shared_owners(step)); + claims + }) + .collect() +} + +/// Returns the paths a job caches under more than one owner. +#[must_use] +pub fn duplicated_paths(job: &Job) -> Vec<(String, Vec)> { + let mut grouped: BTreeMap> = BTreeMap::new(); + for claim in owners_for(job) { + let owners = grouped.entry(claim.path).or_default(); + if !owners.contains(&claim.owner) { + owners.push(claim.owner); + } + } + grouped + .into_iter() + .filter(|(_, owners)| owners.len() > 1) + .collect() +} diff --git a/tests/support/workflow_model.rs b/tests/support/workflow_model.rs new file mode 100644 index 0000000..68b0111 --- /dev/null +++ b/tests/support/workflow_model.rs @@ -0,0 +1,230 @@ +//! Structural model of the repository's GitHub Actions workflow files. +//! +//! The workflow-contract tests assert placement, tool-install, and cache +//! ownership rules over this model rather than over raw YAML text, so a +//! reordered key or a reflowed block scalar cannot silently defeat a rule. +//! +//! # Examples +//! +//! ```no_run +//! let workflows = workflow_model::load_workflows()?; +//! assert!(workflows.iter().any(|w| w.file == "ci.yml")); +//! # Ok::<(), workflow_model::WorkflowError>(()) +//! ``` + +use std::{ + fmt, fs, + path::{Path, PathBuf}, +}; + +use serde_norway::Value; + +/// Directory holding the repository's workflow definitions. +pub const WORKFLOW_DIR: &str = ".github/workflows"; + +/// Commit that every `actions/cache` reference must pin (v6.1.0). +pub const CACHE_ACTION_SHA: &str = "55cc8345863c7cc4c66a329aec7e433d2d1c52a9"; + +/// Commit that every `leynos/shared-actions` reference must pin. +pub const SHARED_ACTIONS_SHA: &str = "7d46a399558914f5a05074e55a560fec0269fd0d"; + +/// Runner label used by this repository's Ubicloud build and test jobs. +pub const UBICLOUD_LABEL: &str = "ubicloud-standard-8"; + +/// Jobs that build or test the crate and therefore keep an Ubicloud label. +pub const BUILD_JOB_IDS: [&str; 2] = ["build-test", "coverage-upload"]; + +/// Failure encountered while reading or parsing the workflow estate. +#[derive(Debug)] +pub enum WorkflowError { + /// A workflow file could not be read. + Read(PathBuf, std::io::Error), + /// A workflow file was not valid YAML. + Parse(PathBuf, serde_norway::Error), + /// A workflow file was structurally unusable. + Shape(PathBuf, String), +} + +impl fmt::Display for WorkflowError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Read(path, err) => write!(f, "cannot read {}: {err}", path.display()), + Self::Parse(path, err) => write!(f, "cannot parse {}: {err}", path.display()), + Self::Shape(path, msg) => write!(f, "unexpected shape in {}: {msg}", path.display()), + } + } +} + +impl std::error::Error for WorkflowError {} + +/// One step of a workflow job, reduced to the fields the contracts inspect. +#[derive(Debug, Clone)] +pub struct Step { + /// Display name, or an empty string when the step is unnamed. + pub name: String, + /// Action reference, or an empty string for a `run` step. + pub uses: String, + /// Shell script, or an empty string for a `uses` step. + pub run: String, + /// Inputs supplied to the action. + pub with: Value, +} + +impl Step { + /// Returns the string value of a `with` input, or an empty string. + #[must_use] + pub fn input(&self, key: &str) -> String { + self.with + .get(key) + .and_then(Value::as_str) + .unwrap_or_default() + .to_owned() + } + + /// Returns the newline-separated `path` input as individual entries. + #[must_use] + pub fn cache_paths(&self) -> Vec { + self.input("path") + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .map(ToOwned::to_owned) + .collect() + } +} + +/// One job of a workflow, reduced to the fields the contracts inspect. +#[derive(Debug, Clone)] +pub struct Job { + /// Key under the workflow's `jobs` mapping. + pub id: String, + /// Runner label, or an empty string when the job calls a reusable workflow. + pub runs_on: String, + /// Reusable workflow reference, or an empty string for a normal job. + pub uses: String, + /// Declared `timeout-minutes`, when present. + pub timeout_minutes: Option, + /// Steps in declaration order. + pub steps: Vec, +} + +impl Job { + /// Reports whether the job runs on a GitHub-hosted Ubuntu runner. + #[must_use] + pub fn is_github_hosted(&self) -> bool { + self.runs_on.starts_with("ubuntu-") + } +} + +/// One workflow file. +#[derive(Debug, Clone)] +pub struct Workflow { + /// File name within [`WORKFLOW_DIR`]. + pub file: String, + /// Jobs in declaration order. + pub jobs: Vec, +} + +fn workflow_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(WORKFLOW_DIR) +} + +fn scalar(value: &Value, key: &str) -> String { + value + .get(key) + .and_then(Value::as_str) + .unwrap_or_default() + .to_owned() +} + +fn parse_step(raw: &Value) -> Step { + Step { + name: scalar(raw, "name"), + uses: scalar(raw, "uses"), + run: scalar(raw, "run"), + with: raw.get("with").cloned().unwrap_or(Value::Null), + } +} + +fn parse_job(id: &str, raw: &Value) -> Job { + let steps = raw + .get("steps") + .and_then(Value::as_sequence) + .map(|items| items.iter().map(parse_step).collect()) + .unwrap_or_default(); + Job { + id: id.to_owned(), + runs_on: scalar(raw, "runs-on"), + uses: scalar(raw, "uses"), + timeout_minutes: raw.get("timeout-minutes").and_then(Value::as_u64), + steps, + } +} + +fn parse_workflow(path: &Path, text: &str) -> Result { + let document: Value = serde_norway::from_str(text) + .map_err(|err| WorkflowError::Parse(path.to_path_buf(), err))?; + let jobs = document + .get("jobs") + .and_then(Value::as_mapping) + .ok_or_else(|| { + WorkflowError::Shape(path.to_path_buf(), "missing a `jobs` mapping".to_owned()) + })?; + let file = path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or_default() + .to_owned(); + let parsed = jobs + .iter() + .map(|(id, raw)| parse_job(id.as_str().unwrap_or_default(), raw)) + .collect(); + Ok(Workflow { file, jobs: parsed }) +} + +/// Loads and parses every workflow in `.github/workflows`. +/// +/// # Errors +/// +/// Returns an error when the directory cannot be listed, a file cannot be +/// read, or a file is not a YAML document containing a `jobs` mapping. +pub fn load_workflows() -> Result, WorkflowError> { + let dir = workflow_dir(); + let listing = fs::read_dir(&dir).map_err(|err| WorkflowError::Read(dir.clone(), err))?; + let mut paths: Vec = Vec::new(); + for entry in listing { + let path = entry + .map_err(|err| WorkflowError::Read(dir.clone(), err))? + .path(); + if path + .extension() + .is_some_and(|ext| ext == "yml" || ext == "yaml") + { + paths.push(path); + } + } + paths.sort(); + paths + .iter() + .map(|path| { + let text = + fs::read_to_string(path).map_err(|err| WorkflowError::Read(path.clone(), err))?; + parse_workflow(path.as_path(), &text) + }) + .collect() +} + +/// Returns every step of every job, tagged with its workflow and job. +#[must_use] +pub fn all_steps(workflows: &[Workflow]) -> Vec<(String, String, Step)> { + workflows + .iter() + .flat_map(|workflow| { + workflow.jobs.iter().flat_map(move |job| { + job.steps + .iter() + .map(move |step| (workflow.file.clone(), job.id.clone(), step.clone())) + }) + }) + .collect() +} diff --git a/tests/workflow_contracts.rs b/tests/workflow_contracts.rs new file mode 100644 index 0000000..788192a --- /dev/null +++ b/tests/workflow_contracts.rs @@ -0,0 +1,243 @@ +//! Structural contracts over the repository's GitHub Actions workflows. +//! +//! These tests encode the Ubicloud adoption rules that a reviewer would +//! otherwise have to re-check by hand on every workflow edit: no tool is built +//! from source, every cached path has exactly one owner, cache and shared +//! action references are pinned, API-bound jobs stay GitHub-hosted, and an +//! installer always precedes the first use of what it installs. +//! +//! They read the workflow files directly, so they fail on the change that +//! introduces a violation rather than on the CI run that suffers from it. + +#[path = "support/workflow_cache_owners.rs"] +mod workflow_cache_owners; +#[path = "support/workflow_model.rs"] +mod workflow_model; + +use std::{fs, path::PathBuf}; + +use rstest::{fixture, rstest}; + +use workflow_model::{ + all_steps, load_workflows, Job, Workflow, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_SHA, + UBICLOUD_LABEL, +}; + +/// Prefixes that mark a step as building a tool from source. +/// +/// `cargo binstall` is included because it compiles whenever its default +/// strategies fall through to `compile`; the estate's rule is to install from +/// a verified release archive instead. +const SOURCE_BUILD_FRAGMENTS: [&str; 3] = ["cargo install", "cargo-binstall ", "cargo binstall"]; + +/// Every workflow in `.github/workflows`, parsed once per test. +#[fixture] +fn workflows() -> Vec { + load_workflows().unwrap_or_else(|err| panic!("workflow estate must parse: {err}")) +} + +fn jobs(workflows: &[Workflow]) -> Vec<(String, Job)> { + workflows + .iter() + .flat_map(|workflow| { + workflow + .jobs + .iter() + .map(move |job| (workflow.file.clone(), job.clone())) + }) + .collect() +} + +fn job_named<'a>(workflows: &'a [Workflow], id: &str) -> &'a Job { + workflows + .iter() + .flat_map(|workflow| workflow.jobs.iter()) + .find(|job| job.id == id) + .unwrap_or_else(|| panic!("workflow estate must define the `{id}` job")) +} + +fn first_step_index(job: &Job, predicate: impl Fn(&str) -> bool) -> Option { + job.steps + .iter() + .position(|step| predicate(&step.run) || predicate(&step.uses)) +} + +#[rstest] +fn every_cache_reference_is_pinned_to_v6_1_0(workflows: Vec) { + let unpinned: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, _, step)| step.uses.starts_with("actions/cache")) + .filter(|(_, _, step)| !step.uses.ends_with(CACHE_ACTION_SHA)) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.uses)) + .collect(); + assert!( + unpinned.is_empty(), + "every actions/cache reference must pin {CACHE_ACTION_SHA} (v6.1.0): {unpinned:?}" + ); +} + +#[rstest] +fn no_workflow_uses_the_ubicloud_cache_fork(workflows: Vec) { + let forks: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, _, step)| step.uses.starts_with("ubicloud/cache")) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.uses)) + .collect(); + assert!( + forks.is_empty(), + "the deprecated ubicloud/cache fork must not be used: {forks:?}" + ); +} + +#[rstest] +fn every_shared_action_reference_is_pinned(workflows: Vec) { + let mut references: Vec = all_steps(&workflows) + .into_iter() + .map(|(file, job, step)| (file, job, step.uses)) + .chain( + jobs(&workflows) + .into_iter() + .map(|(file, job)| (file, job.id.clone(), job.uses)), + ) + .filter(|(_, _, uses)| uses.starts_with("leynos/shared-actions")) + .filter(|(_, _, uses)| !uses.ends_with(SHARED_ACTIONS_SHA)) + .map(|(file, job, uses)| format!("{file}:{job}: {uses}")) + .collect(); + references.sort(); + assert!( + references.is_empty(), + "every leynos/shared-actions reference must pin {SHARED_ACTIONS_SHA}: {references:?}" + ); +} + +#[rstest] +fn no_step_builds_a_tool_from_source(workflows: Vec) { + let offenders: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, _, step)| { + SOURCE_BUILD_FRAGMENTS + .iter() + .any(|fragment| step.run.contains(fragment)) + }) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) + .collect(); + assert!( + offenders.is_empty(), + "tools must be installed from verified prebuilt releases, not compiled: {offenders:?}" + ); +} + +#[rstest] +fn install_action_fails_closed_rather_than_compiling(workflows: Vec) { + let permissive: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, _, step)| step.uses.starts_with("taiki-e/install-action")) + .filter(|(_, _, step)| step.input("fallback") != "none") + .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) + .collect(); + assert!( + permissive.is_empty(), + "taiki-e/install-action must set `fallback: none` so it cannot compile: {permissive:?}" + ); +} + +#[rstest] +fn each_cached_path_has_exactly_one_owner(workflows: Vec) { + let clashes: Vec = jobs(&workflows) + .into_iter() + .flat_map(|(file, job)| { + workflow_cache_owners::duplicated_paths(&job) + .into_iter() + .map(move |(path, owners)| format!("{file}:{}: {path} owned by {owners:?}", job.id)) + }) + .collect(); + assert!( + clashes.is_empty(), + "each cached path must have one owner: {clashes:?}" + ); +} + +#[rstest] +fn non_build_jobs_stay_on_github_hosted_runners(workflows: Vec) { + let misplaced: Vec = jobs(&workflows) + .into_iter() + .filter(|(_, job)| !job.runs_on.is_empty()) + .filter(|(_, job)| !BUILD_JOB_IDS.contains(&job.id.as_str())) + .filter(|(_, job)| !job.is_github_hosted()) + .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) + .collect(); + assert!( + misplaced.is_empty(), + "delayed-comment, metadata, and other API-bound jobs must stay GitHub-hosted: {misplaced:?}" + ); +} + +#[rstest] +fn build_jobs_keep_their_ubicloud_label_and_a_timeout(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + assert_eq!( + job.runs_on, UBICLOUD_LABEL, + "`{id}` must keep its measured runner label" + ); + assert!( + job.timeout_minutes.is_some(), + "`{id}` bills by the minute and must declare timeout-minutes" + ); + } +} + +#[rstest] +fn every_runner_label_is_registered_with_actionlint(workflows: Vec) { + let config = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(".github/actionlint.yaml"); + let text = fs::read_to_string(&config).unwrap_or_default(); + let unregistered: Vec = jobs(&workflows) + .into_iter() + .filter(|(_, job)| !job.runs_on.is_empty() && !job.is_github_hosted()) + .filter(|(_, job)| !text.contains(job.runs_on.as_str())) + .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) + .collect(); + assert!( + unregistered.is_empty(), + "every self-hosted label must appear in .github/actionlint.yaml: {unregistered:?}" + ); +} + +#[rstest] +#[case::rust_toolchain("setup-rust", "cargo")] +#[case::whitaker_suite("install-whitaker", "whitaker ")] +fn an_installer_precedes_the_first_use_of_its_tool( + workflows: Vec, + #[case] installer: &str, + #[case] first_use: &str, +) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let install_at = first_step_index(job, |text| text.contains(installer)); + let use_at = first_step_index(job, |text| text.contains(first_use)); + let Some(use_index) = use_at else { continue }; + let Some(install_index) = install_at else { + panic!("`{id}` uses `{first_use}` without a `{installer}` step"); + }; + assert!( + install_index < use_index, + "`{id}` must run `{installer}` before step {use_index} uses `{first_use}`" + ); + } +} + +#[rstest] +fn coverage_is_the_only_linux_test_execution(workflows: Vec) { + let duplicates: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, job, _)| BUILD_JOB_IDS.contains(&job.as_str())) + .filter(|(_, _, step)| { + step.run.contains("cargo test") || step.run.contains("cargo nextest") + }) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) + .collect(); + assert!( + duplicates.is_empty(), + "the instrumented coverage run is the only test execution; drop the repeat: {duplicates:?}" + ); +} From 6ab148b99836cfabd7700e1acdc6e9587b515230 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 3 Sep 2026 23:22:02 +0100 Subject: [PATCH 03/10] Document CI placement, installers, and cache owners Record why `build-test` and `coverage-upload` sit on a paid runner while every API-bound job stays GitHub-hosted, which action installs each tool, which step owns each cached path, and why two downloads stay uncached on purpose. Without this a future contributor reads the cache steps as arbitrary and either duplicates an owner or removes one that pays. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- docs/developers-guide.md | 74 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 18c9c37..6a02203 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -339,3 +339,77 @@ buffered-message compile-pass harness `make lint` runs rustdoc (`--cfg docsrs`), `cargo clippy --all-targets --all-features -- -D warnings`, and the Whitaker Dylint suite. + + +## Continuous integration + +Two workflows do the developer-blocking work. `ci.yml`'s `build-test` job runs +on every pull request, and `coverage-main.yml`'s `coverage-upload` job runs on +every push to `main`. Both use the `ubicloud-standard-8` runner label, which is +registered in `.github/actionlint.yaml`, and both declare `timeout-minutes` so +a hung step cannot bill to the platform's six-hour default. + +Every other job stays on GitHub-hosted `ubuntu-latest`. That placement is a +rule, not an accident: delayed comments, metadata lookups, label handling, and +release orchestration are API-bound, so paid runner capacity buys them nothing +and their queue time is already short. `dependabot-automerge.yml` calls a +reusable workflow, which chooses its own runner. + + +### Tool installation + +No tool is compiled from source. `whitaker-installer` is installed by +`leynos/shared-actions/.github/actions/install-whitaker`, which downloads the +pinned prebuilt release archive and verifies it against a digest pinned inside +the action, then runs the installer to place the Whitaker Dylint suite. Every +`leynos/shared-actions` reference pins commit +`7d46a399558914f5a05074e55a560fec0269fd0d`. + + +### Cache ownership + +Each mutable path has exactly one owner, so no two steps race to write it and +every miss is explainable from the rendered key. + +| Path | Owner | Key inputs | +| -------------------------------------------------------------------------------- | --------------------------------------------- | ------------------------------------------------------------------------------------ | +| `~/.cargo/registry`, `~/.cargo/git` | `setup-rust` (`cache-provider: github`) | `runner.os`, `rust-toolchain.toml` and `Cargo.lock` hash | +| `~/.cargo/bin/whitaker-installer`, its version marker, `~/.local/share/whitaker` | `install-whitaker` (`cache-provider: github`) | `runner.os`, `runner.arch`, installer version, `dylint.toml` hash | +| `.uv-cache`, `.uv-tools` | the `Cache uv tool layers` step in `ci.yml` | `runner.os`, `runner.arch`, `runner.environment`, `Makefile` and `scripts/*.py` hash | +| coverage ratchet baseline files | `generate-coverage`'s split restore and save | `runner.os`, run id | + +`generate-coverage` is called with `cache-provider: external` in both jobs +because `setup-rust` already owns the Cargo registry and Git index; without +that input the action would become a second owner of the same two paths. For +the same reason no step archives a `target` tree, and `actions/cache` is pinned +to `55cc8345863c7cc4c66a329aec7e433d2d1c52a9` (v6.1.0) everywhere. + +`use-sccache: 'false'` is passed to `setup-rust` because nothing in this +repository sets `RUSTC_WRAPPER`. Installing sccache would download a binary +that serves no compilation and has no cache owner. Adopting a compiler cache is +a separate, measured change. + +Two downloads remain deliberately uncached. The `cs-coverage` CLI is fetched on +every run because `upload-codescene-coverage` only caches it when `cli-version` +is pinned, and its cache step uses an unpinned `actions/cache@v4` that this +repository cannot pin from here. The uv tool layers are cached by the +pull-request job that installs them, which is also the only job that installs +them; there is no trunk job to designate as the sole writer instead. + + +### One test execution per pull request + +The instrumented coverage run is the only test execution on Linux. It uses +`all-features`, `all-targets`, and `doctests`, so it covers everything the +former separate `cargo test` step covered and more, for one compile rather than +two. A workflow contract in `tests/workflow_contracts.rs` fails if a second +`cargo test` or `cargo nextest` step reappears in either job. + + +### Workflow contracts + +`tests/workflow_contracts.rs` parses the workflow files and asserts the rules +above: pinned cache and shared-action references, no source-built tools, one +owner per cached path, GitHub-hosted placement for non-build jobs, registered +runner labels, and an installer before the first use of what it installs. Run +them with `make test`, and run `actionlint` after editing any workflow. From f4b7c03db9fd29f771c1fecaed4a6911ef882a9b Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 3 Sep 2026 23:43:25 +0100 Subject: [PATCH 04/10] Address review of the workflow contracts and guide Identify each cache claim by the position of the step that makes it, not by the step's display name. GitHub Actions lets two steps in one job share a name, so name-based identity let a second owner of a path disappear into the first and the ownership contract could pass a workflow it should reject. The one deliberate collapse remains: an `actions/cache/restore` step and an `actions/cache/save` step that share a key are the two halves of one owner, so they report one identity. Read the workflow files through a `cap_std` directory capability rooted at `.github/workflows` instead of ambient `std::fs` paths, in line with the repository's filesystem rule. One ambient call opens the directory; every listing and read goes through the handle and cannot leave it. Caption the cache-ownership table, and scope the pinning claim in the guide. The contracts check the workflow files, so "everywhere" overstated them: a shared action can reach an `actions/cache` reference of its own, and `upload-codescene-coverage` does. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- Cargo.toml | 5 ++ docs/developers-guide.md | 10 ++- tests/support/workflow_cache_owners.rs | 60 +++++++++------ tests/support/workflow_model.rs | 101 +++++++++++++------------ 4 files changed, 100 insertions(+), 76 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dc81e7d..ec3d380 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -120,6 +120,11 @@ rspec = "1.0" # tests/workflow_contracts.rs. A maintained fork of the unmaintained # serde_yaml, so serde derives and Value keep working. serde_norway = "0.9" +# Capability-scoped filesystem access for the workflow-contract loader, so it +# reads through a directory handle rooted at .github/workflows rather than +# ambient std::fs paths. +cap-std = { version = "4", features = ["fs_utf8"] } +camino = "1" test_utils = { path = "test_utils" } trybuild = "1.0" # Non-optional in dev builds so the `trybuild` compile-pass fixture, which is a diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 6a02203..b640c7b 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -378,11 +378,16 @@ every miss is explainable from the rendered key. | `.uv-cache`, `.uv-tools` | the `Cache uv tool layers` step in `ci.yml` | `runner.os`, `runner.arch`, `runner.environment`, `Makefile` and `scripts/*.py` hash | | coverage ratchet baseline files | `generate-coverage`'s split restore and save | `runner.os`, run id | +*Table 1: Cache ownership and cache-key inputs.* + `generate-coverage` is called with `cache-provider: external` in both jobs because `setup-rust` already owns the Cargo registry and Git index; without that input the action would become a second owner of the same two paths. For -the same reason no step archives a `target` tree, and `actions/cache` is pinned -to `55cc8345863c7cc4c66a329aec7e433d2d1c52a9` (v6.1.0) everywhere. +the same reason no step archives a `target` tree. Every `actions/cache` +reference written in these workflow files pins +`55cc8345863c7cc4c66a329aec7e433d2d1c52a9` (v6.1.0). That claim covers the +workflow files only. A shared action may reach an `actions/cache` reference of +its own, and `upload-codescene-coverage` does; the next paragraph records it. `use-sccache: 'false'` is passed to `setup-rust` because nothing in this repository sets `RUSTC_WRAPPER`. Installing sccache would download a binary @@ -396,7 +401,6 @@ repository cannot pin from here. The uv tool layers are cached by the pull-request job that installs them, which is also the only job that installs them; there is no trunk job to designate as the sole writer instead. - ### One test execution per pull request The instrumented coverage run is the only test execution on Linux. It uses diff --git a/tests/support/workflow_cache_owners.rs b/tests/support/workflow_cache_owners.rs index 4801f7f..1974a69 100644 --- a/tests/support/workflow_cache_owners.rs +++ b/tests/support/workflow_cache_owners.rs @@ -6,6 +6,12 @@ //! the path. This module reduces both kinds to the same `(path, owner)` list //! so one contract can compare them. //! +//! Owner identity is the step's position in its job, never its display name: +//! two steps may legitimately share a name, and collapsing them would hide a +//! duplicate owner. The one deliberate exception is a split cache, where an +//! `actions/cache/restore` step and an `actions/cache/save` step that share a +//! key are the two halves of a single owner. +//! //! # Examples //! //! ```no_run @@ -22,7 +28,7 @@ use crate::workflow_model::{Job, Step}; pub struct CacheOwner { /// Cached path, as written in the workflow or the shared action. pub path: String, - /// Step that claims the path, named for a readable assertion message. + /// Identity of the claiming owner, unique per step or per split-cache key. pub owner: String, } @@ -55,30 +61,39 @@ const SHARED_ACTION_CACHES: [(&str, &[&str]); 3] = [ ]; fn shared_action_name(uses: &str) -> Option<&str> { - let path = uses.split('@').next()?; - let name = path.strip_prefix("leynos/shared-actions/.github/actions/")?; - Some(name) + uses.split('@') + .next()? + .strip_prefix("leynos/shared-actions/.github/actions/") } -fn is_cache_action(uses: &str) -> bool { - uses.split('@') - .next() - .is_some_and(|path| path == "actions/cache" || path.starts_with("actions/cache/")) +fn action_path(uses: &str) -> &str { + uses.split('@').next().unwrap_or_default() } -fn step_label(step: &Step) -> String { - if step.name.is_empty() { - step.uses.clone() +/// Identity of the owner making a claim. +/// +/// A whole-cache or shared-action claim is owned by its step alone. A split +/// cache is owned jointly by the restore and save steps that share its key, +/// so both halves report the same identity and are not counted twice. +fn owner_identity(step: &Step, index: usize) -> String { + let label = if step.name.is_empty() { + step.uses.as_str() } else { - step.name.clone() + step.name.as_str() + }; + match action_path(&step.uses) { + "actions/cache/restore" | "actions/cache/save" => { + format!("split cache with key `{}`", step.input("key")) + } + _ => format!("step {index} (`{label}`)"), } } -fn direct_owners(step: &Step) -> Vec { - if !is_cache_action(&step.uses) { +fn direct_owners(step: &Step, index: usize) -> Vec { + if !action_path(&step.uses).starts_with("actions/cache") { return Vec::new(); } - let owner = step_label(step); + let owner = owner_identity(step, index); step.cache_paths() .into_iter() .map(|path| CacheOwner { @@ -88,7 +103,7 @@ fn direct_owners(step: &Step) -> Vec { .collect() } -fn shared_owners(step: &Step) -> Vec { +fn shared_owners(step: &Step, index: usize) -> Vec { let Some(name) = shared_action_name(&step.uses) else { return Vec::new(); }; @@ -98,7 +113,7 @@ fn shared_owners(step: &Step) -> Vec { if !provider.is_empty() && provider != "github" { return Vec::new(); } - let owner = step_label(step); + let owner = owner_identity(step, index); SHARED_ACTION_CACHES .iter() .filter(|(action, _)| *action == name) @@ -111,17 +126,14 @@ fn shared_owners(step: &Step) -> Vec { } /// Returns every cache claim made by a job, in step order. -/// -/// `actions/cache/restore` and `actions/cache/save` halves of one split cache -/// share a step name prefix in practice; they are reported separately and the -/// caller decides whether the pair is a duplicate. #[must_use] pub fn owners_for(job: &Job) -> Vec { job.steps .iter() - .flat_map(|step| { - let mut claims = direct_owners(step); - claims.extend(shared_owners(step)); + .enumerate() + .flat_map(|(index, step)| { + let mut claims = direct_owners(step, index); + claims.extend(shared_owners(step, index)); claims }) .collect() diff --git a/tests/support/workflow_model.rs b/tests/support/workflow_model.rs index 68b0111..7aec324 100644 --- a/tests/support/workflow_model.rs +++ b/tests/support/workflow_model.rs @@ -4,6 +4,10 @@ //! ownership rules over this model rather than over raw YAML text, so a //! reordered key or a reflowed block scalar cannot silently defeat a rule. //! +//! Files are read through a `cap_std` directory capability rooted at +//! `.github/workflows`, so the loader cannot reach outside the workflow +//! directory even if a future contract passes it a name it should not. +//! //! # Examples //! //! ```no_run @@ -12,11 +16,10 @@ //! # Ok::<(), workflow_model::WorkflowError>(()) //! ``` -use std::{ - fmt, fs, - path::{Path, PathBuf}, -}; +use std::fmt; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs_utf8::Dir}; use serde_norway::Value; /// Directory holding the repository's workflow definitions. @@ -37,20 +40,20 @@ pub const BUILD_JOB_IDS: [&str; 2] = ["build-test", "coverage-upload"]; /// Failure encountered while reading or parsing the workflow estate. #[derive(Debug)] pub enum WorkflowError { - /// A workflow file could not be read. - Read(PathBuf, std::io::Error), + /// A workflow file or the workflow directory could not be read. + Read(String, std::io::Error), /// A workflow file was not valid YAML. - Parse(PathBuf, serde_norway::Error), + Parse(String, serde_norway::Error), /// A workflow file was structurally unusable. - Shape(PathBuf, String), + Shape(String, String), } impl fmt::Display for WorkflowError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Read(path, err) => write!(f, "cannot read {}: {err}", path.display()), - Self::Parse(path, err) => write!(f, "cannot parse {}: {err}", path.display()), - Self::Shape(path, msg) => write!(f, "unexpected shape in {}: {msg}", path.display()), + Self::Read(name, err) => write!(f, "cannot read {name}: {err}"), + Self::Parse(name, err) => write!(f, "cannot parse {name}: {err}"), + Self::Shape(name, msg) => write!(f, "unexpected shape in {name}: {msg}"), } } } @@ -125,10 +128,6 @@ pub struct Workflow { pub jobs: Vec, } -fn workflow_dir() -> PathBuf { - PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(WORKFLOW_DIR) -} - fn scalar(value: &Value, key: &str) -> String { value .get(key) @@ -161,55 +160,59 @@ fn parse_job(id: &str, raw: &Value) -> Job { } } -fn parse_workflow(path: &Path, text: &str) -> Result { - let document: Value = serde_norway::from_str(text) - .map_err(|err| WorkflowError::Parse(path.to_path_buf(), err))?; - let jobs = document +fn parse_workflow(file: &str, text: &str) -> Result { + let document: Value = + serde_norway::from_str(text).map_err(|err| WorkflowError::Parse(file.to_owned(), err))?; + let raw_jobs = document .get("jobs") .and_then(Value::as_mapping) .ok_or_else(|| { - WorkflowError::Shape(path.to_path_buf(), "missing a `jobs` mapping".to_owned()) + WorkflowError::Shape(file.to_owned(), "missing a `jobs` mapping".to_owned()) })?; - let file = path - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or_default() - .to_owned(); - let parsed = jobs + let jobs = raw_jobs .iter() .map(|(id, raw)| parse_job(id.as_str().unwrap_or_default(), raw)) .collect(); - Ok(Workflow { file, jobs: parsed }) + Ok(Workflow { + file: file.to_owned(), + jobs, + }) +} + +fn workflow_names(dir: &Dir) -> Result, WorkflowError> { + let read = |err| WorkflowError::Read(WORKFLOW_DIR.to_owned(), err); + let mut names: Vec = Vec::new(); + for entry in dir.entries().map_err(read)? { + let name = entry.map_err(read)?.file_name().map_err(read)?; + let extension = Utf8Path::new(&name).extension().unwrap_or_default(); + if matches!(extension, "yml" | "yaml") { + names.push(name); + } + } + names.sort(); + Ok(names) } /// Loads and parses every workflow in `.github/workflows`. /// /// # Errors /// -/// Returns an error when the directory cannot be listed, a file cannot be -/// read, or a file is not a YAML document containing a `jobs` mapping. +/// Returns an error when the workflow directory cannot be opened or listed, a +/// file cannot be read, or a file is not a YAML document containing a `jobs` +/// mapping. pub fn load_workflows() -> Result, WorkflowError> { - let dir = workflow_dir(); - let listing = fs::read_dir(&dir).map_err(|err| WorkflowError::Read(dir.clone(), err))?; - let mut paths: Vec = Vec::new(); - for entry in listing { - let path = entry - .map_err(|err| WorkflowError::Read(dir.clone(), err))? - .path(); - if path - .extension() - .is_some_and(|ext| ext == "yml" || ext == "yaml") - { - paths.push(path); - } - } - paths.sort(); - paths + let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(WORKFLOW_DIR); + // The one ambient step: everything below reads through this capability, + // which cannot escape the workflow directory. + let dir = Dir::open_ambient_dir(&root, ambient_authority()) + .map_err(|err| WorkflowError::Read(root.to_string(), err))?; + workflow_names(&dir)? .iter() - .map(|path| { - let text = - fs::read_to_string(path).map_err(|err| WorkflowError::Read(path.clone(), err))?; - parse_workflow(path.as_path(), &text) + .map(|name| { + let text = dir + .read_to_string(name) + .map_err(|err| WorkflowError::Read(name.clone(), err))?; + parse_workflow(name, &text) }) .collect() } From ecaf21660ab5ccc0876d273ceb0da826c29c7416 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 3 Sep 2026 23:55:11 +0100 Subject: [PATCH 05/10] Guard the workflow inputs and sample the ownership rules The contracts checked the shape of the CI policy but not its substance. A workflow could drop `cache-provider: external` from the coverage step, pin a different Whitaker installer, cache one uv layer instead of two, or set a five-minute timeout, and every test would still pass. Assert the inputs that make the rules true: the cache provider and sccache setting on `setup-rust`, the installer version and provider on `install-whitaker`, the three coverage flags and the external provider on `generate-coverage`, the exact uv layer paths and the runner and source-hash fragments its key must carry, and a bounded `timeout-minutes` for each build job. Widen the single-execution rule to the repository's own test targets, not only literal cargo commands. Parse strictly. A field that is present but of the wrong type was becoming an empty string, so a mistyped `runs-on` or a malformed `with` mapping could pass a contract that should have rejected it. Every scalar field now fails with the workflow, job, and field named, and six malformed documents are tested alongside an unreadable workflow directory. Booleans and numbers are rendered the way GitHub passes them to an action, so `doctests: true` and `doctests: 'true'` still compare equal. Reading the actionlint configuration now surfaces its error instead of substituting empty input, which would have made the label contract vacuously pass. Add sampled properties over the model, per ADR 003, which admits `proptest` alongside the bounded matrices for broader domains. The ownership and ordering rules hold over arbitrary step orderings, repeated display names, interleaved unrelated steps, and split caches whose halves agree or disagree on a key. Each property is checked against a small oracle written independently of the implementation. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- docs/developers-guide.md | 25 +- tests/support/workflow_cache_owners.rs | 3 + tests/support/workflow_model.rs | 260 ++++++++++++++---- tests/workflow_contracts.rs | 185 ++++++++++--- ...flow_model_properties.proptest-regressions | 8 + tests/workflow_model_properties.rs | 208 ++++++++++++++ 6 files changed, 606 insertions(+), 83 deletions(-) create mode 100644 tests/workflow_model_properties.proptest-regressions create mode 100644 tests/workflow_model_properties.rs diff --git a/docs/developers-guide.md b/docs/developers-guide.md index b640c7b..bdbd84e 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -415,5 +415,26 @@ two. A workflow contract in `tests/workflow_contracts.rs` fails if a second `tests/workflow_contracts.rs` parses the workflow files and asserts the rules above: pinned cache and shared-action references, no source-built tools, one owner per cached path, GitHub-hosted placement for non-build jobs, registered -runner labels, and an installer before the first use of what it installs. Run -them with `make test`, and run `actionlint` after editing any workflow. +runner labels, an installer before the first use of what it installs, and a +single test execution per build job. It also pins the inputs that make those +rules true, so a workflow cannot keep the shape of the policy while dropping +its substance: `cache-provider`, `use-sccache`, the Whitaker installer +version, the coverage flags, the uv cache paths and key, and a bounded +`timeout-minutes` for each build job. + +Parsing is strict. A workflow field that is present but of the wrong type is +an error rather than a silent default, because a contract that read an empty +string for a mistyped `runs-on` would pass a workflow it should reject. The +files are read through a `cap_std` directory capability rooted at +`.github/workflows`. + +Two assurance methods are used together, following +[ADR 003](adr-003-bounded-rstest-over-property-testing.md). +`tests/workflow_contracts.rs` holds bounded `rstest` cases over the workflow +files as they stand, and `tests/workflow_model_properties.rs` samples the +wider domain with `proptest`: arbitrary step orderings, repeated display +names, interleaved unrelated steps, and split caches whose halves agree or +disagree on a key. The properties check cache-owner uniqueness and +installer-ordering against small oracles written independently of the +implementation. Run both with `make test`, and run `actionlint` after editing +any workflow. diff --git a/tests/support/workflow_cache_owners.rs b/tests/support/workflow_cache_owners.rs index 1974a69..45dcb26 100644 --- a/tests/support/workflow_cache_owners.rs +++ b/tests/support/workflow_cache_owners.rs @@ -60,6 +60,7 @@ const SHARED_ACTION_CACHES: [(&str, &[&str]); 3] = [ ), ]; +/// Returns the shared-action name a `uses` reference names, if any. fn shared_action_name(uses: &str) -> Option<&str> { uses.split('@') .next()? @@ -89,6 +90,7 @@ fn owner_identity(step: &Step, index: usize) -> String { } } +/// Returns the claims an `actions/cache` step makes on its own `path` input. fn direct_owners(step: &Step, index: usize) -> Vec { if !action_path(&step.uses).starts_with("actions/cache") { return Vec::new(); @@ -103,6 +105,7 @@ fn direct_owners(step: &Step, index: usize) -> Vec { .collect() } +/// Returns the claims a shared composite action makes on the caller's behalf. fn shared_owners(step: &Step, index: usize) -> Vec { let Some(name) = shared_action_name(&step.uses) else { return Vec::new(); diff --git a/tests/support/workflow_model.rs b/tests/support/workflow_model.rs index 7aec324..72ae520 100644 --- a/tests/support/workflow_model.rs +++ b/tests/support/workflow_model.rs @@ -4,6 +4,11 @@ //! ownership rules over this model rather than over raw YAML text, so a //! reordered key or a reflowed block scalar cannot silently defeat a rule. //! +//! Parsing is strict about shape. A field that is present but of the wrong +//! type is an error rather than a silent default, because a contract that +//! reads an empty string for a mistyped `runs-on` would pass a workflow it +//! should reject. Defaults are used only where a field is genuinely optional. +//! //! Files are read through a `cap_std` directory capability rooted at //! `.github/workflows`, so the loader cannot reach outside the workflow //! directory even if a future contract passes it a name it should not. @@ -16,7 +21,7 @@ //! # Ok::<(), workflow_model::WorkflowError>(()) //! ``` -use std::fmt; +use std::{collections::BTreeMap, fmt}; use camino::{Utf8Path, Utf8PathBuf}; use cap_std::{ambient_authority, fs_utf8::Dir}; @@ -49,6 +54,7 @@ pub enum WorkflowError { } impl fmt::Display for WorkflowError { + /// Renders the failure with the workflow name that produced it. fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Read(name, err) => write!(f, "cannot read {name}: {err}"), @@ -60,8 +66,54 @@ impl fmt::Display for WorkflowError { impl std::error::Error for WorkflowError {} +/// Builds a shape error naming the offending workflow and field. +fn shape(context: &str, message: &str) -> WorkflowError { + WorkflowError::Shape(context.to_owned(), message.to_owned()) +} + +/// Renders a YAML scalar as the string a workflow expression would see. +/// +/// GitHub Actions coerces booleans and numbers to strings when it passes an +/// input to an action, so `doctests: true` and `doctests: 'true'` reach the +/// action identically and must compare equal here too. +fn render_scalar(value: &Value) -> Option { + match value { + Value::String(text) => Some(text.clone()), + Value::Bool(flag) => Some(flag.to_string()), + Value::Number(number) => Some(number.to_string()), + _ => None, + } +} + +/// Reads an optional string field, defaulting to an empty string. +/// +/// # Errors +/// +/// Returns an error when the field is present but is not a scalar. +fn optional_string(raw: &Value, key: &str, context: &str) -> Result { + let Some(value) = raw.get(key) else { + return Ok(String::new()); + }; + render_scalar(value).ok_or_else(|| shape(context, &format!("`{key}` must be a scalar"))) +} + +/// Reads an optional unsigned integer field. +/// +/// # Errors +/// +/// Returns an error when the field is present but is not an unsigned integer. +fn optional_u64(raw: &Value, key: &str, context: &str) -> Result, WorkflowError> { + let Some(value) = raw.get(key) else { + return Ok(None); + }; + value + .as_u64() + .map(Some) + .ok_or_else(|| shape(context, &format!("`{key}` must be an unsigned integer"))) +} + /// One step of a workflow job, reduced to the fields the contracts inspect. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct Step { /// Display name, or an empty string when the step is unnamed. pub name: String, @@ -69,19 +121,18 @@ pub struct Step { pub uses: String, /// Shell script, or an empty string for a `uses` step. pub run: String, - /// Inputs supplied to the action. - pub with: Value, + /// Inputs supplied to the action, rendered as GitHub would pass them. + pub with: BTreeMap, } impl Step { - /// Returns the string value of a `with` input, or an empty string. + /// Returns the value of a `with` input, or an empty string when absent. + /// + /// Every input was validated as a scalar during parsing, so an absent + /// input and a mistyped one cannot be confused here. #[must_use] - pub fn input(&self, key: &str) -> String { - self.with - .get(key) - .and_then(Value::as_str) - .unwrap_or_default() - .to_owned() + pub fn input(&self, key: &str) -> &str { + self.with.get(key).map_or("", String::as_str) } /// Returns the newline-separated `path` input as individual entries. @@ -94,10 +145,20 @@ impl Step { .map(ToOwned::to_owned) .collect() } + + /// Returns the step's display name, falling back to its action reference. + #[must_use] + pub const fn label(&self) -> &str { + if self.name.is_empty() { + self.uses.as_str() + } else { + self.name.as_str() + } + } } /// One job of a workflow, reduced to the fields the contracts inspect. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct Job { /// Key under the workflow's `jobs` mapping. pub id: String, @@ -117,6 +178,25 @@ impl Job { pub fn is_github_hosted(&self) -> bool { self.runs_on.starts_with("ubuntu-") } + + /// Returns the first step whose `run` or `uses` text contains `needle`. + #[must_use] + pub fn first_step_containing(&self, needle: &str) -> Option { + self.steps + .iter() + .position(|step| step.run.contains(needle) || step.uses.contains(needle)) + } + + /// Returns the first step whose `uses` names `action`, ignoring its pin. + #[must_use] + pub fn step_using(&self, action: &str) -> Option<&Step> { + self.steps.iter().find(|step| { + step.uses + .split('@') + .next() + .is_some_and(|path| path.ends_with(action)) + }) + } } /// One workflow file. @@ -128,57 +208,117 @@ pub struct Workflow { pub jobs: Vec, } -fn scalar(value: &Value, key: &str) -> String { - value - .get(key) - .and_then(Value::as_str) - .unwrap_or_default() - .to_owned() -} - -fn parse_step(raw: &Value) -> Step { - Step { - name: scalar(raw, "name"), - uses: scalar(raw, "uses"), - run: scalar(raw, "run"), - with: raw.get("with").cloned().unwrap_or(Value::Null), +/// Parses a step's `with` mapping into rendered scalar inputs. +/// +/// # Errors +/// +/// Returns an error when `with` is not a mapping or an input is not a scalar. +fn parse_inputs(raw: &Value, context: &str) -> Result, WorkflowError> { + let Some(value) = raw.get("with") else { + return Ok(BTreeMap::new()); + }; + let mapping = value + .as_mapping() + .ok_or_else(|| shape(context, "`with` must be a mapping"))?; + mapping + .iter() + .map(|(key, item)| { + let name = key + .as_str() + .ok_or_else(|| shape(context, "every `with` key must be a string"))?; + let rendered = render_scalar(item) + .ok_or_else(|| shape(context, &format!("input `{name}` must be a scalar")))?; + Ok((name.to_owned(), rendered)) + }) + .collect() +} + +/// Parses one step of a job. +/// +/// # Errors +/// +/// Returns an error when the step is not a mapping, has a mistyped field, or +/// neither runs a script nor uses an action. +fn parse_step(raw: &Value, context: &str) -> Result { + if raw.as_mapping().is_none() { + return Err(shape(context, "every step must be a mapping")); } + let step = Step { + name: optional_string(raw, "name", context)?, + uses: optional_string(raw, "uses", context)?, + run: optional_string(raw, "run", context)?, + with: parse_inputs(raw, context)?, + }; + if step.uses.is_empty() && step.run.is_empty() { + return Err(shape(context, "every step must set `uses` or `run`")); + } + Ok(step) } -fn parse_job(id: &str, raw: &Value) -> Job { - let steps = raw - .get("steps") - .and_then(Value::as_sequence) - .map(|items| items.iter().map(parse_step).collect()) - .unwrap_or_default(); - Job { +/// Parses one job of a workflow. +/// +/// # Errors +/// +/// Returns an error when a field is mistyped, `steps` is not a sequence, or +/// the job neither names a runner nor calls a reusable workflow. +fn parse_job(id: &str, raw: &Value, file: &str) -> Result { + let context = format!("{file}: job `{id}`"); + let steps = match raw.get("steps") { + None => Vec::new(), + Some(value) => value + .as_sequence() + .ok_or_else(|| shape(&context, "`steps` must be a sequence"))? + .iter() + .map(|step| parse_step(step, &context)) + .collect::, WorkflowError>>()?, + }; + let job = Job { id: id.to_owned(), - runs_on: scalar(raw, "runs-on"), - uses: scalar(raw, "uses"), - timeout_minutes: raw.get("timeout-minutes").and_then(Value::as_u64), + runs_on: optional_string(raw, "runs-on", &context)?, + uses: optional_string(raw, "uses", &context)?, + timeout_minutes: optional_u64(raw, "timeout-minutes", &context)?, steps, + }; + if job.runs_on.is_empty() && job.uses.is_empty() { + return Err(shape(&context, "a job must set `runs-on` or `uses`")); } + Ok(job) } -fn parse_workflow(file: &str, text: &str) -> Result { +/// Parses one workflow document. +/// +/// # Errors +/// +/// Returns an error when the text is not YAML, has no `jobs` mapping, or +/// contains a job or step of unexpected shape. +pub fn parse_workflow(file: &str, text: &str) -> Result { let document: Value = serde_norway::from_str(text).map_err(|err| WorkflowError::Parse(file.to_owned(), err))?; let raw_jobs = document .get("jobs") .and_then(Value::as_mapping) - .ok_or_else(|| { - WorkflowError::Shape(file.to_owned(), "missing a `jobs` mapping".to_owned()) - })?; + .ok_or_else(|| shape(file, "missing a `jobs` mapping"))?; let jobs = raw_jobs .iter() - .map(|(id, raw)| parse_job(id.as_str().unwrap_or_default(), raw)) - .collect(); + .map(|(key, raw)| { + let id = key + .as_str() + .ok_or_else(|| shape(file, "every job id must be a string"))?; + parse_job(id, raw, file) + }) + .collect::, WorkflowError>>()?; Ok(Workflow { file: file.to_owned(), jobs, }) } +/// Lists the workflow file names inside an opened workflow directory. +/// +/// # Errors +/// +/// Returns an error when the directory cannot be listed or an entry's name +/// cannot be read. fn workflow_names(dir: &Dir) -> Result, WorkflowError> { let read = |err| WorkflowError::Read(WORKFLOW_DIR.to_owned(), err); let mut names: Vec = Vec::new(); @@ -193,18 +333,16 @@ fn workflow_names(dir: &Dir) -> Result, WorkflowError> { Ok(names) } -/// Loads and parses every workflow in `.github/workflows`. +/// Loads and parses every workflow beneath `root`. /// /// # Errors /// -/// Returns an error when the workflow directory cannot be opened or listed, a -/// file cannot be read, or a file is not a YAML document containing a `jobs` -/// mapping. -pub fn load_workflows() -> Result, WorkflowError> { - let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(WORKFLOW_DIR); +/// Returns an error when the directory cannot be opened or listed, a file +/// cannot be read, or a file is not a workflow document. +pub fn load_workflows_in(root: &Utf8Path) -> Result, WorkflowError> { // The one ambient step: everything below reads through this capability, // which cannot escape the workflow directory. - let dir = Dir::open_ambient_dir(&root, ambient_authority()) + let dir = Dir::open_ambient_dir(root, ambient_authority()) .map_err(|err| WorkflowError::Read(root.to_string(), err))?; workflow_names(&dir)? .iter() @@ -217,6 +355,30 @@ pub fn load_workflows() -> Result, WorkflowError> { .collect() } +/// Loads and parses every workflow in this repository's `.github/workflows`. +/// +/// # Errors +/// +/// Returns the same errors as [`load_workflows_in`]. +pub fn load_workflows() -> Result, WorkflowError> { + let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(WORKFLOW_DIR); + load_workflows_in(&root) +} + +/// Reads a file from this repository's root through a directory capability. +/// +/// # Errors +/// +/// Returns an error when the repository root cannot be opened or the file +/// cannot be read. +pub fn read_repository_file(relative: &str) -> Result { + let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let dir = Dir::open_ambient_dir(&root, ambient_authority()) + .map_err(|err| WorkflowError::Read(root.to_string(), err))?; + dir.read_to_string(relative) + .map_err(|err| WorkflowError::Read(relative.to_owned(), err)) +} + /// Returns every step of every job, tagged with its workflow and job. #[must_use] pub fn all_steps(workflows: &[Workflow]) -> Vec<(String, String, Step)> { diff --git a/tests/workflow_contracts.rs b/tests/workflow_contracts.rs index 788192a..5109b65 100644 --- a/tests/workflow_contracts.rs +++ b/tests/workflow_contracts.rs @@ -4,7 +4,9 @@ //! otherwise have to re-check by hand on every workflow edit: no tool is built //! from source, every cached path has exactly one owner, cache and shared //! action references are pinned, API-bound jobs stay GitHub-hosted, and an -//! installer always precedes the first use of what it installs. +//! installer always precedes the first use of what it installs. They also pin +//! the inputs that make those rules true, so a workflow cannot keep the shape +//! of the policy while dropping its substance. //! //! They read the workflow files directly, so they fail on the change that //! introduces a violation rather than on the CI run that suffers from it. @@ -14,28 +16,39 @@ mod workflow_cache_owners; #[path = "support/workflow_model.rs"] mod workflow_model; -use std::{fs, path::PathBuf}; - +use camino::Utf8Path; use rstest::{fixture, rstest}; use workflow_model::{ - all_steps, load_workflows, Job, Workflow, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_SHA, - UBICLOUD_LABEL, + all_steps, load_workflows, load_workflows_in, parse_workflow, read_repository_file, Job, Step, + Workflow, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_SHA, UBICLOUD_LABEL, }; -/// Prefixes that mark a step as building a tool from source. +/// Fragments that mark a step as building a tool from source. /// /// `cargo binstall` is included because it compiles whenever its default /// strategies fall through to `compile`; the estate's rule is to install from /// a verified release archive instead. const SOURCE_BUILD_FRAGMENTS: [&str; 3] = ["cargo install", "cargo-binstall ", "cargo binstall"]; +/// Commands that would run the test suite a second time in a build job. +const REPEAT_TEST_COMMANDS: [&str; 4] = ["cargo test", "cargo nextest", "make test", "make all"]; + +/// Expression fragments the uv tool-layer cache key must carry. +const UV_CACHE_KEY_FRAGMENTS: [&str; 4] = [ + "runner.os", + "runner.arch", + "runner.environment", + "hashFiles(", +]; + /// Every workflow in `.github/workflows`, parsed once per test. #[fixture] fn workflows() -> Vec { load_workflows().unwrap_or_else(|err| panic!("workflow estate must parse: {err}")) } +/// Returns every job in the estate, tagged with its workflow file. fn jobs(workflows: &[Workflow]) -> Vec<(String, Job)> { workflows .iter() @@ -48,6 +61,7 @@ fn jobs(workflows: &[Workflow]) -> Vec<(String, Job)> { .collect() } +/// Returns the job with the given id, or panics naming the missing job. fn job_named<'a>(workflows: &'a [Workflow], id: &str) -> &'a Job { workflows .iter() @@ -56,10 +70,20 @@ fn job_named<'a>(workflows: &'a [Workflow], id: &str) -> &'a Job { .unwrap_or_else(|| panic!("workflow estate must define the `{id}` job")) } -fn first_step_index(job: &Job, predicate: impl Fn(&str) -> bool) -> Option { - job.steps - .iter() - .position(|step| predicate(&step.run) || predicate(&step.uses)) +/// Returns a job's step that uses `action`, or panics naming both. +fn step_using<'a>(job: &'a Job, action: &str) -> &'a Step { + job.step_using(action) + .unwrap_or_else(|| panic!("`{}` must use the `{action}` action", job.id)) +} + +/// Asserts that a step supplies the expected value for one input. +fn assert_input(job_id: &str, step: &Step, key: &str, expected: &str) { + assert_eq!( + step.input(key), + expected, + "`{job_id}` step `{}` must set `{key}: {expected}`", + step.label() + ); } #[rstest] @@ -172,25 +196,37 @@ fn non_build_jobs_stay_on_github_hosted_runners(workflows: Vec) { ); } +/// The measured bounds for each build job's `timeout-minutes`. +/// +/// The lower bound keeps the timeout above the observed median so a normal run +/// cannot be killed; the upper bound keeps a hung run from billing for hours. #[rstest] -fn build_jobs_keep_their_ubicloud_label_and_a_timeout(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - assert_eq!( - job.runs_on, UBICLOUD_LABEL, - "`{id}` must keep its measured runner label" - ); - assert!( - job.timeout_minutes.is_some(), - "`{id}` bills by the minute and must declare timeout-minutes" - ); - } +#[case::build_test("build-test", 45, 120)] +#[case::coverage_upload("coverage-upload", 30, 90)] +fn build_jobs_keep_their_label_and_a_bounded_timeout( + workflows: Vec, + #[case] id: &str, + #[case] lowest: u64, + #[case] highest: u64, +) { + let job = job_named(&workflows, id); + assert_eq!( + job.runs_on, UBICLOUD_LABEL, + "`{id}` must keep its measured runner label" + ); + let timeout = job + .timeout_minutes + .unwrap_or_else(|| panic!("`{id}` bills by the minute and must declare timeout-minutes")); + assert!( + (lowest..=highest).contains(&timeout), + "`{id}` timeout-minutes {timeout} must lie between {lowest} and {highest}" + ); } #[rstest] fn every_runner_label_is_registered_with_actionlint(workflows: Vec) { - let config = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(".github/actionlint.yaml"); - let text = fs::read_to_string(&config).unwrap_or_default(); + let text = read_repository_file(".github/actionlint.yaml") + .unwrap_or_else(|err| panic!("actionlint configuration must be readable: {err}")); let unregistered: Vec = jobs(&workflows) .into_iter() .filter(|(_, job)| !job.runs_on.is_empty() && !job.is_github_hosted()) @@ -213,12 +249,12 @@ fn an_installer_precedes_the_first_use_of_its_tool( ) { for id in BUILD_JOB_IDS { let job = job_named(&workflows, id); - let install_at = first_step_index(job, |text| text.contains(installer)); - let use_at = first_step_index(job, |text| text.contains(first_use)); - let Some(use_index) = use_at else { continue }; - let Some(install_index) = install_at else { - panic!("`{id}` uses `{first_use}` without a `{installer}` step"); + let Some(use_index) = job.first_step_containing(first_use) else { + continue; }; + let install_index = job + .first_step_containing(installer) + .unwrap_or_else(|| panic!("`{id}` uses `{first_use}` without a `{installer}` step")); assert!( install_index < use_index, "`{id}` must run `{installer}` before step {use_index} uses `{first_use}`" @@ -227,17 +263,102 @@ fn an_installer_precedes_the_first_use_of_its_tool( } #[rstest] -fn coverage_is_the_only_linux_test_execution(workflows: Vec) { +fn coverage_is_the_only_test_execution(workflows: Vec) { let duplicates: Vec = all_steps(&workflows) .into_iter() .filter(|(_, job, _)| BUILD_JOB_IDS.contains(&job.as_str())) .filter(|(_, _, step)| { - step.run.contains("cargo test") || step.run.contains("cargo nextest") + REPEAT_TEST_COMMANDS + .iter() + .any(|command| step.run.contains(command)) }) - .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.label())) .collect(); assert!( duplicates.is_empty(), "the instrumented coverage run is the only test execution; drop the repeat: {duplicates:?}" ); } + +#[rstest] +fn setup_rust_owns_the_cargo_registry_and_installs_no_compiler_cache(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let step = step_using(job, "setup-rust"); + assert_input(id, step, "cache-provider", "github"); + assert_input(id, step, "use-sccache", "false"); + } +} + +#[rstest] +fn coverage_runs_the_whole_suite_once_and_owns_no_cargo_cache(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let step = step_using(job, "generate-coverage"); + for flag in ["all-features", "all-targets", "doctests"] { + assert_input(id, step, flag, "true"); + } + assert_input(id, step, "cache-provider", "external"); + } +} + +#[rstest] +fn whitaker_is_installed_from_a_pinned_prebuilt_release(workflows: Vec) { + let job = job_named(&workflows, "build-test"); + let step = step_using(job, "install-whitaker"); + assert_input("build-test", step, "installer-version", "0.2.7"); + assert_input("build-test", step, "cache-provider", "github"); +} + +#[rstest] +fn the_uv_cache_names_its_layers_and_keys_them_by_runner(workflows: Vec) { + let job = job_named(&workflows, "build-test"); + let step = job + .steps + .iter() + .find(|step| step.cache_paths().iter().any(|path| path == ".uv-cache")) + .unwrap_or_else(|| panic!("`build-test` must cache the uv download layer")); + assert_eq!( + step.cache_paths(), + vec![".uv-cache".to_owned(), ".uv-tools".to_owned()], + "the uv cache must own both the download store and the tool store" + ); + let key = step.input("key"); + for fragment in UV_CACHE_KEY_FRAGMENTS { + assert!( + key.contains(fragment), + "the uv cache key `{key}` must vary with `{fragment}`" + ); + } +} + +#[rstest] +#[case::not_a_workflow("scratch.yml", "steps: []")] +#[case::mistyped_runner("scratch.yml", "jobs:\n a:\n runs-on: [a, b]\n")] +#[case::placeless_job("scratch.yml", "jobs:\n a:\n steps: []\n")] +#[case::mistyped_steps("scratch.yml", "jobs:\n a:\n runs-on: x\n steps: nope\n")] +#[case::empty_step( + "scratch.yml", + "jobs:\n a:\n runs-on: x\n steps:\n - name: n\n" +)] +#[case::mistyped_input( + "scratch.yml", + "jobs:\n a:\n runs-on: x\n steps:\n - uses: u\n with:\n k: [1]\n" +)] +fn a_malformed_workflow_is_an_error_not_a_default(#[case] file: &str, #[case] text: &str) { + let outcome = parse_workflow(file, text); + assert!( + outcome.is_err(), + "a workflow of unexpected shape must be rejected, not silently defaulted" + ); +} + +#[rstest] +fn an_unreadable_workflow_directory_is_reported() { + let missing = Utf8Path::new("this/directory/does/not/exist"); + let outcome = load_workflows_in(missing); + assert!( + outcome.is_err(), + "an unreadable workflow directory must surface as an error" + ); +} diff --git a/tests/workflow_model_properties.proptest-regressions b/tests/workflow_model_properties.proptest-regressions new file mode 100644 index 0000000..1c94c63 --- /dev/null +++ b/tests/workflow_model_properties.proptest-regressions @@ -0,0 +1,8 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc eda260dbf3c89826f7936ca408a99a86ed1493f61547e8cc4a9a11da51c2abff # shrinks to external = true, filler = [Step { name: "Cache", uses: "actions/cache@sha", run: "", with: {"path": "~/.cargo/registry"} }] +cc 993efbb5ed20e4bd09be1cce19290b1d88ccac2146b5d2bb8326d595e4dc6239 # shrinks to path = 2, same_key = true, filler = [Step { name: "Cache", uses: "actions/cache@sha", run: "", with: {"path": ".uv-cache"} }] diff --git a/tests/workflow_model_properties.rs b/tests/workflow_model_properties.rs new file mode 100644 index 0000000..5296c57 --- /dev/null +++ b/tests/workflow_model_properties.rs @@ -0,0 +1,208 @@ +//! Sampled properties over the workflow model's ownership and ordering rules. +//! +//! The deterministic contracts in `workflow_contracts.rs` check the current +//! workflow files. They cannot show that the cache-ownership and step-ordering +//! logic behaves over the wider domain of jobs a future edit could produce: +//! arbitrary step orderings, repeated display names, interleaved unrelated +//! steps, and split caches whose halves agree or disagree on a key. Per +//! `docs/adr-003-bounded-rstest-over-property-testing.md`, `proptest` +//! supplements the bounded matrices for exactly that kind of broader domain. +//! +//! Each property is checked against a small oracle expressed independently of +//! the implementation, rather than by re-deriving the implementation's answer. + +#[path = "support/workflow_cache_owners.rs"] +mod workflow_cache_owners; +#[path = "support/workflow_model.rs"] +// The model is shared with tests/workflow_contracts.rs, which exercises the +// loading and contract-facing half. These properties need only the job and +// step types, so the rest is unused in this binary alone. +#[expect( + dead_code, + reason = "shared support module; the contracts binary uses the rest" +)] +mod workflow_model; + +use std::collections::{BTreeMap, BTreeSet}; + +use proptest::prelude::*; + +use workflow_cache_owners::duplicated_paths; +use workflow_model::{Job, Step}; + +/// Cache paths the generators draw from, kept small so collisions are common. +const PATHS: [&str; 4] = ["~/.cargo/registry", "~/.cargo/git", ".uv-cache", "target-x"]; + +/// Display names the generators draw from, including deliberate repeats. +const NAMES: [&str; 3] = ["Cache", "Cache", "Restore"]; + +/// Builds a step that uses an action with the given inputs. +fn action_step(name: &str, uses: &str, inputs: &[(&str, &str)]) -> Step { + Step { + name: name.to_owned(), + uses: uses.to_owned(), + run: String::new(), + with: inputs + .iter() + .map(|(key, value)| ((*key).to_owned(), (*value).to_owned())) + .collect::>(), + } +} + +/// Builds a whole-cache step claiming one path. +fn cache_step(name: &str, path: &str) -> Step { + action_step(name, "actions/cache@sha", &[("path", path)]) +} + +/// Builds one half of a split cache claiming one path under one key. +fn split_step(half: &str, path: &str, key: &str) -> Step { + action_step( + "Split", + &format!("actions/cache/{half}@sha"), + &[("path", path), ("key", key)], + ) +} + +/// Builds a step that runs a shell command and caches nothing. +fn run_step(script: &str) -> Step { + Step { + run: script.to_owned(), + ..Step::default() + } +} + +/// Wraps steps in a job that satisfies the model's shape requirements. +fn job_of(steps: Vec) -> Job { + Job { + id: "j".to_owned(), + runs_on: "ubuntu-latest".to_owned(), + steps, + ..Job::default() + } +} + +/// Generates a step that either caches a path or does unrelated work. +fn any_step() -> impl Strategy { + prop_oneof![ + ( + prop::sample::select(NAMES.to_vec()), + prop::sample::select(PATHS.to_vec()), + ) + .prop_map(|(name, path)| cache_step(name, path)), + any::().prop_map(|flag| run_step(if flag { "make lint" } else { "echo hello" })), + ] +} + +/// Returns the set of paths claimed more than once, ignoring owner identity. +/// +/// This oracle counts claiming steps directly, so it is independent of how +/// the implementation names an owner. +fn paths_claimed_twice(job: &Job) -> BTreeSet { + let mut seen: BTreeMap = BTreeMap::new(); + for step in &job.steps { + if step.uses.starts_with("actions/cache@") { + for path in step.cache_paths() { + *seen.entry(path).or_default() += 1; + } + } + } + seen.into_iter() + .filter(|(_, count)| *count > 1) + .map(|(path, _)| path) + .collect() +} + +/// Drops filler steps that would themselves claim the path under test. +/// +/// The filler exists to prove that unrelated steps between two claims do not +/// disturb the result; a filler that claims the same path would instead test +/// a different scenario. +fn without_claims_on(steps: Vec, path: &str) -> Vec { + steps + .into_iter() + .filter(|step| !step.cache_paths().iter().any(|claimed| claimed == path)) + .collect() +} + +/// Returns the reported duplicated paths as a set. +fn reported(job: &Job) -> BTreeSet { + duplicated_paths(job) + .into_iter() + .map(|(path, _)| path) + .collect() +} + +proptest! { + /// Two whole-cache steps claiming a path are always two owners, whatever + /// their display names, positions, or the steps interleaved between them. + #[test] + fn repeated_whole_cache_claims_are_always_duplicates(steps in prop::collection::vec(any_step(), 0..8)) { + let job = job_of(steps); + prop_assert_eq!(reported(&job), paths_claimed_twice(&job)); + } + + /// Reordering a job's steps cannot change which paths have two owners. + #[test] + fn duplicate_detection_ignores_step_order( + steps in prop::collection::vec(any_step(), 0..8), + rotation in 0usize..8, + ) { + let job = job_of(steps.clone()); + let mut rotated = steps; + let count = rotated.len(); + if count > 0 { + rotated.rotate_left(rotation % count); + } + prop_assert_eq!(reported(&job), reported(&job_of(rotated))); + } + + /// A restore and a save sharing a key are one owner; differing keys are two. + #[test] + fn a_split_cache_is_one_owner_only_when_its_halves_agree( + path in prop::sample::select(PATHS.to_vec()), + same_key in any::(), + filler in prop::collection::vec(any_step(), 0..4), + ) { + let save_key = if same_key { "k1" } else { "k2" }; + let mut steps = vec![split_step("restore", path, "k1")]; + steps.extend(without_claims_on(filler, path)); + steps.push(split_step("save", path, save_key)); + let job = job_of(steps); + prop_assert_eq!(reported(&job).contains(path), !same_key); + } + + /// A shared action is an owner exactly when the caller has not taken its + /// paths with `cache-provider: external`. + #[test] + fn an_external_cache_provider_removes_the_shared_action_as_an_owner( + external in any::(), + filler in prop::collection::vec(any_step(), 0..4), + ) { + let provider = if external { "external" } else { "github" }; + let mut steps = vec![cache_step("Registry", "~/.cargo/registry")]; + steps.extend(without_claims_on(filler, "~/.cargo/registry")); + steps.push(action_step( + "Setup Rust", + "leynos/shared-actions/.github/actions/setup-rust@sha", + &[("cache-provider", provider)], + )); + let job = job_of(steps); + prop_assert_eq!(reported(&job).contains("~/.cargo/registry"), !external); + } + + /// `first_step_containing` always returns the least matching index. + #[test] + fn the_first_matching_step_is_the_least_matching_index( + scripts in prop::collection::vec(prop::sample::select(vec!["whitaker --all", "cargo test", "echo"]), 0..8), + ) { + let job = job_of(scripts.iter().map(|script| run_step(script)).collect()); + let expected = job + .steps + .iter() + .enumerate() + .filter(|(_, step)| step.run.contains("whitaker")) + .map(|(index, _)| index) + .min(); + prop_assert_eq!(job.first_step_containing("whitaker"), expected); + } +} From 4b7f18f68619089a0de3ca5b6d43bf644dfa7390 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Fri, 4 Sep 2026 00:03:47 +0100 Subject: [PATCH 06/10] Name the parse location instead of passing strings CodeScene flagged the workflow model for String Heavy Function Arguments. Several parsing helpers took a bare `&str` context alongside a `&str` field name, so a call site could swap the two and still compile, and the error message would name the wrong thing. Introduce `Location`, which knows how to descend from a file to a job and how to build a shape error at that point, and `WorkflowSource`, which pairs a document with the file name it came from. Each helper now takes at most one string argument, and `parse_workflow` takes none. Split the module while doing so. `workflow_model.rs` holds the types and the queries the contracts ask of them; `workflow_loader.rs` reads and parses files into those types. The combined file had passed 400 lines, and the two halves have different audiences: the sampled properties need only the types. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- docs/developers-guide.md | 12 +- tests/support/workflow_loader.rs | 255 ++++++++++++++++++++++++++++ tests/support/workflow_model.rs | 276 ++++--------------------------- tests/workflow_contracts.rs | 11 +- 4 files changed, 303 insertions(+), 251 deletions(-) create mode 100644 tests/support/workflow_loader.rs diff --git a/docs/developers-guide.md b/docs/developers-guide.md index bdbd84e..e0d55fd 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -412,7 +412,7 @@ two. A workflow contract in `tests/workflow_contracts.rs` fails if a second ### Workflow contracts -`tests/workflow_contracts.rs` parses the workflow files and asserts the rules +`tests/workflow_contracts.rs` asserts the rules above: pinned cache and shared-action references, no source-built tools, one owner per cached path, GitHub-hosted placement for non-build jobs, registered runner labels, an installer before the first use of what it installs, and a @@ -422,10 +422,12 @@ its substance: `cache-provider`, `use-sccache`, the Whitaker installer version, the coverage flags, the uv cache paths and key, and a bounded `timeout-minutes` for each build job. -Parsing is strict. A workflow field that is present but of the wrong type is -an error rather than a silent default, because a contract that read an empty -string for a mistyped `runs-on` would pass a workflow it should reject. The -files are read through a `cap_std` directory capability rooted at +`tests/support/workflow_model.rs` holds the types and the queries the +contracts ask of them, and `tests/support/workflow_loader.rs` turns files into +those values. Parsing is strict: a workflow field that is present but of the +wrong type is an error rather than a silent default, because a contract that +read an empty string for a mistyped `runs-on` would pass a workflow it should +reject. The files are read through a `cap_std` directory capability rooted at `.github/workflows`. Two assurance methods are used together, following diff --git a/tests/support/workflow_loader.rs b/tests/support/workflow_loader.rs new file mode 100644 index 0000000..a74f285 --- /dev/null +++ b/tests/support/workflow_loader.rs @@ -0,0 +1,255 @@ +//! Reads and parses the repository's GitHub Actions workflow files. +//! +//! Parsing is strict about shape. A field that is present but of the wrong +//! type is an error rather than a silent default, because a contract that +//! read an empty string for a mistyped `runs-on` would pass a workflow it +//! should reject. Defaults are used only where a field is genuinely optional. +//! +//! Files are read through a `cap_std` directory capability rooted at the +//! directory being loaded, so the loader cannot reach outside it even if a +//! future contract passes it a name it should not. +//! +//! # Examples +//! +//! ```no_run +//! let workflows = workflow_loader::load_workflows()?; +//! assert!(workflows.iter().any(|w| w.file == "ci.yml")); +//! # Ok::<(), workflow_model::WorkflowError>(()) +//! ``` + +use std::collections::BTreeMap; + +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use serde_norway::Value; + +use crate::workflow_model::{ + Job, Location, Step, Workflow, WorkflowError, WorkflowSource, WORKFLOW_DIR, +}; + +/// Renders a YAML scalar as the string a workflow expression would see. +/// +/// GitHub Actions coerces booleans and numbers to strings when it passes an +/// input to an action, so `doctests: true` and `doctests: 'true'` reach the +/// action identically and must compare equal here too. +fn render_scalar(value: &Value) -> Option { + match value { + Value::String(text) => Some(text.clone()), + Value::Bool(flag) => Some(flag.to_string()), + Value::Number(number) => Some(number.to_string()), + _ => None, + } +} + +/// Reads an optional string field, defaulting to an empty string. +/// +/// # Errors +/// +/// Returns an error when the field is present but is not a scalar. +fn optional_string(raw: &Value, key: &str, at: &Location) -> Result { + let Some(value) = raw.get(key) else { + return Ok(String::new()); + }; + render_scalar(value).ok_or_else(|| at.shape(&format!("`{key}` must be a scalar"))) +} + +/// Reads an optional unsigned integer field. +/// +/// # Errors +/// +/// Returns an error when the field is present but is not an unsigned integer. +fn optional_u64(raw: &Value, key: &str, at: &Location) -> Result, WorkflowError> { + let Some(value) = raw.get(key) else { + return Ok(None); + }; + value + .as_u64() + .map(Some) + .ok_or_else(|| at.shape(&format!("`{key}` must be an unsigned integer"))) +} + +/// Returns an error when `with` is not a mapping or an input is not a scalar. +fn parse_inputs(raw: &Value, at: &Location) -> Result, WorkflowError> { + let Some(value) = raw.get("with") else { + return Ok(BTreeMap::new()); + }; + let mapping = value + .as_mapping() + .ok_or_else(|| at.shape("`with` must be a mapping"))?; + mapping + .iter() + .map(|(key, item)| { + let name = key + .as_str() + .ok_or_else(|| at.shape("every `with` key must be a string"))?; + let rendered = render_scalar(item) + .ok_or_else(|| at.shape(&format!("input `{name}` must be a scalar")))?; + Ok((name.to_owned(), rendered)) + }) + .collect() +} + +/// Parses one step of a job. +/// +/// # Errors +/// +/// Returns an error when the step is not a mapping, has a mistyped field, or +/// neither runs a script nor uses an action. +fn parse_step(raw: &Value, at: &Location) -> Result { + if raw.as_mapping().is_none() { + return Err(at.shape("every step must be a mapping")); + } + let step = Step { + name: optional_string(raw, "name", at)?, + uses: optional_string(raw, "uses", at)?, + run: optional_string(raw, "run", at)?, + with: parse_inputs(raw, at)?, + }; + if step.uses.is_empty() && step.run.is_empty() { + return Err(at.shape("every step must set `uses` or `run`")); + } + Ok(step) +} + +/// Parses one job of a workflow. +/// +/// # Errors +/// +/// Returns an error when a field is mistyped, `steps` is not a sequence, or +/// the job neither names a runner nor calls a reusable workflow. +fn parse_job(id: &str, raw: &Value, file: &Location) -> Result { + let at = file.job(id); + let steps = match raw.get("steps") { + None => Vec::new(), + Some(value) => value + .as_sequence() + .ok_or_else(|| at.shape("`steps` must be a sequence"))? + .iter() + .map(|step| parse_step(step, &at)) + .collect::, WorkflowError>>()?, + }; + let job = Job { + id: id.to_owned(), + runs_on: optional_string(raw, "runs-on", &at)?, + uses: optional_string(raw, "uses", &at)?, + timeout_minutes: optional_u64(raw, "timeout-minutes", &at)?, + steps, + }; + if job.runs_on.is_empty() && job.uses.is_empty() { + return Err(at.shape("a job must set `runs-on` or `uses`")); + } + Ok(job) +} + +/// Parses one workflow document. +/// +/// # Errors +/// +/// Returns an error when the text is not YAML, has no `jobs` mapping, or +/// contains a job or step of unexpected shape. +pub fn parse_workflow(source: WorkflowSource<'_>) -> Result { + let at = Location::file(source.file); + let document: Value = serde_norway::from_str(source.text) + .map_err(|err| WorkflowError::Parse(source.file.to_owned(), err))?; + let raw_jobs = document + .get("jobs") + .and_then(Value::as_mapping) + .ok_or_else(|| at.shape("missing a `jobs` mapping"))?; + let jobs = raw_jobs + .iter() + .map(|(key, raw)| { + let id = key + .as_str() + .ok_or_else(|| at.shape("every job id must be a string"))?; + parse_job(id, raw, &at) + }) + .collect::, WorkflowError>>()?; + Ok(Workflow { + file: source.file.to_owned(), + jobs, + }) +} + +/// Lists the workflow file names inside an opened workflow directory. +/// +/// # Errors +/// +/// Returns an error when the directory cannot be listed or an entry's name +/// cannot be read. +fn workflow_names(dir: &Dir) -> Result, WorkflowError> { + let read = |err| WorkflowError::Read(WORKFLOW_DIR.to_owned(), err); + let mut names: Vec = Vec::new(); + for entry in dir.entries().map_err(read)? { + let name = entry.map_err(read)?.file_name().map_err(read)?; + let extension = Utf8Path::new(&name).extension().unwrap_or_default(); + if matches!(extension, "yml" | "yaml") { + names.push(name); + } + } + names.sort(); + Ok(names) +} + +/// Loads and parses every workflow beneath `root`. +/// +/// # Errors +/// +/// Returns an error when the directory cannot be opened or listed, a file +/// cannot be read, or a file is not a workflow document. +pub fn load_workflows_in(root: &Utf8Path) -> Result, WorkflowError> { + // The one ambient step: everything below reads through this capability, + // which cannot escape the workflow directory. + let dir = Dir::open_ambient_dir(root, ambient_authority()) + .map_err(|err| WorkflowError::Read(root.to_string(), err))?; + workflow_names(&dir)? + .iter() + .map(|name| { + let text = dir + .read_to_string(name) + .map_err(|err| WorkflowError::Read(name.clone(), err))?; + parse_workflow(WorkflowSource { + file: name, + text: &text, + }) + }) + .collect() +} + +/// Loads and parses every workflow in this repository's `.github/workflows`. +/// +/// # Errors +/// +/// Returns the same errors as [`load_workflows_in`]. +pub fn load_workflows() -> Result, WorkflowError> { + let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(WORKFLOW_DIR); + load_workflows_in(&root) +} + +/// Reads a file from this repository's root through a directory capability. +/// +/// # Errors +/// +/// Returns an error when the repository root cannot be opened or the file +/// cannot be read. +pub fn read_repository_file(relative: &str) -> Result { + let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let dir = Dir::open_ambient_dir(&root, ambient_authority()) + .map_err(|err| WorkflowError::Read(root.to_string(), err))?; + dir.read_to_string(relative) + .map_err(|err| WorkflowError::Read(relative.to_owned(), err)) +} + +/// Returns every step of every job, tagged with its workflow and job. +#[must_use] +pub fn all_steps(workflows: &[Workflow]) -> Vec<(String, String, Step)> { + workflows + .iter() + .flat_map(|workflow| { + workflow.jobs.iter().flat_map(move |job| { + job.steps + .iter() + .map(move |step| (workflow.file.clone(), job.id.clone(), step.clone())) + }) + }) + .collect() +} diff --git a/tests/support/workflow_model.rs b/tests/support/workflow_model.rs index 72ae520..c81d449 100644 --- a/tests/support/workflow_model.rs +++ b/tests/support/workflow_model.rs @@ -1,32 +1,20 @@ -//! Structural model of the repository's GitHub Actions workflow files. +//! Types describing the repository's GitHub Actions workflow estate. //! //! The workflow-contract tests assert placement, tool-install, and cache -//! ownership rules over this model rather than over raw YAML text, so a +//! ownership rules over these types rather than over raw YAML text, so a //! reordered key or a reflowed block scalar cannot silently defeat a rule. -//! -//! Parsing is strict about shape. A field that is present but of the wrong -//! type is an error rather than a silent default, because a contract that -//! reads an empty string for a mistyped `runs-on` would pass a workflow it -//! should reject. Defaults are used only where a field is genuinely optional. -//! -//! Files are read through a `cap_std` directory capability rooted at -//! `.github/workflows`, so the loader cannot reach outside the workflow -//! directory even if a future contract passes it a name it should not. +//! `workflow_loader` turns files into these values; this module holds only +//! the shapes and the queries the contracts ask of them. //! //! # Examples //! //! ```no_run -//! let workflows = workflow_model::load_workflows()?; -//! assert!(workflows.iter().any(|w| w.file == "ci.yml")); -//! # Ok::<(), workflow_model::WorkflowError>(()) +//! let job = workflow_model::Job::default(); +//! assert!(!job.is_github_hosted()); //! ``` use std::{collections::BTreeMap, fmt}; -use camino::{Utf8Path, Utf8PathBuf}; -use cap_std::{ambient_authority, fs_utf8::Dir}; -use serde_norway::Value; - /// Directory holding the repository's workflow definitions. pub const WORKFLOW_DIR: &str = ".github/workflows"; @@ -66,50 +54,38 @@ impl fmt::Display for WorkflowError { impl std::error::Error for WorkflowError {} -/// Builds a shape error naming the offending workflow and field. -fn shape(context: &str, message: &str) -> WorkflowError { - WorkflowError::Shape(context.to_owned(), message.to_owned()) -} +/// Where in the estate a value was read, carried instead of a bare string so +/// the parsing helpers take one string argument rather than several. +#[derive(Debug, Clone)] +pub struct Location(String); -/// Renders a YAML scalar as the string a workflow expression would see. -/// -/// GitHub Actions coerces booleans and numbers to strings when it passes an -/// input to an action, so `doctests: true` and `doctests: 'true'` reach the -/// action identically and must compare equal here too. -fn render_scalar(value: &Value) -> Option { - match value { - Value::String(text) => Some(text.clone()), - Value::Bool(flag) => Some(flag.to_string()), - Value::Number(number) => Some(number.to_string()), - _ => None, +impl Location { + /// Locates a whole workflow file. + #[must_use] + pub fn file(name: &str) -> Self { + Self(name.to_owned()) } -} -/// Reads an optional string field, defaulting to an empty string. -/// -/// # Errors -/// -/// Returns an error when the field is present but is not a scalar. -fn optional_string(raw: &Value, key: &str, context: &str) -> Result { - let Some(value) = raw.get(key) else { - return Ok(String::new()); - }; - render_scalar(value).ok_or_else(|| shape(context, &format!("`{key}` must be a scalar"))) + /// Locates one job within this file. + #[must_use] + pub fn job(&self, id: &str) -> Self { + Self(format!("{}: job `{id}`", self.0)) + } + + /// Builds a shape error reported at this location. + #[must_use] + pub fn shape(&self, message: &str) -> WorkflowError { + WorkflowError::Shape(self.0.clone(), message.to_owned()) + } } -/// Reads an optional unsigned integer field. -/// -/// # Errors -/// -/// Returns an error when the field is present but is not an unsigned integer. -fn optional_u64(raw: &Value, key: &str, context: &str) -> Result, WorkflowError> { - let Some(value) = raw.get(key) else { - return Ok(None); - }; - value - .as_u64() - .map(Some) - .ok_or_else(|| shape(context, &format!("`{key}` must be an unsigned integer"))) +/// A workflow document paired with the file name it came from. +#[derive(Debug, Clone, Copy)] +pub struct WorkflowSource<'a> { + /// File name within [`WORKFLOW_DIR`]. + pub file: &'a str, + /// The document's YAML text. + pub text: &'a str, } /// One step of a workflow job, reduced to the fields the contracts inspect. @@ -207,189 +183,3 @@ pub struct Workflow { /// Jobs in declaration order. pub jobs: Vec, } - -/// Parses a step's `with` mapping into rendered scalar inputs. -/// -/// # Errors -/// -/// Returns an error when `with` is not a mapping or an input is not a scalar. -fn parse_inputs(raw: &Value, context: &str) -> Result, WorkflowError> { - let Some(value) = raw.get("with") else { - return Ok(BTreeMap::new()); - }; - let mapping = value - .as_mapping() - .ok_or_else(|| shape(context, "`with` must be a mapping"))?; - mapping - .iter() - .map(|(key, item)| { - let name = key - .as_str() - .ok_or_else(|| shape(context, "every `with` key must be a string"))?; - let rendered = render_scalar(item) - .ok_or_else(|| shape(context, &format!("input `{name}` must be a scalar")))?; - Ok((name.to_owned(), rendered)) - }) - .collect() -} - -/// Parses one step of a job. -/// -/// # Errors -/// -/// Returns an error when the step is not a mapping, has a mistyped field, or -/// neither runs a script nor uses an action. -fn parse_step(raw: &Value, context: &str) -> Result { - if raw.as_mapping().is_none() { - return Err(shape(context, "every step must be a mapping")); - } - let step = Step { - name: optional_string(raw, "name", context)?, - uses: optional_string(raw, "uses", context)?, - run: optional_string(raw, "run", context)?, - with: parse_inputs(raw, context)?, - }; - if step.uses.is_empty() && step.run.is_empty() { - return Err(shape(context, "every step must set `uses` or `run`")); - } - Ok(step) -} - -/// Parses one job of a workflow. -/// -/// # Errors -/// -/// Returns an error when a field is mistyped, `steps` is not a sequence, or -/// the job neither names a runner nor calls a reusable workflow. -fn parse_job(id: &str, raw: &Value, file: &str) -> Result { - let context = format!("{file}: job `{id}`"); - let steps = match raw.get("steps") { - None => Vec::new(), - Some(value) => value - .as_sequence() - .ok_or_else(|| shape(&context, "`steps` must be a sequence"))? - .iter() - .map(|step| parse_step(step, &context)) - .collect::, WorkflowError>>()?, - }; - let job = Job { - id: id.to_owned(), - runs_on: optional_string(raw, "runs-on", &context)?, - uses: optional_string(raw, "uses", &context)?, - timeout_minutes: optional_u64(raw, "timeout-minutes", &context)?, - steps, - }; - if job.runs_on.is_empty() && job.uses.is_empty() { - return Err(shape(&context, "a job must set `runs-on` or `uses`")); - } - Ok(job) -} - -/// Parses one workflow document. -/// -/// # Errors -/// -/// Returns an error when the text is not YAML, has no `jobs` mapping, or -/// contains a job or step of unexpected shape. -pub fn parse_workflow(file: &str, text: &str) -> Result { - let document: Value = - serde_norway::from_str(text).map_err(|err| WorkflowError::Parse(file.to_owned(), err))?; - let raw_jobs = document - .get("jobs") - .and_then(Value::as_mapping) - .ok_or_else(|| shape(file, "missing a `jobs` mapping"))?; - let jobs = raw_jobs - .iter() - .map(|(key, raw)| { - let id = key - .as_str() - .ok_or_else(|| shape(file, "every job id must be a string"))?; - parse_job(id, raw, file) - }) - .collect::, WorkflowError>>()?; - Ok(Workflow { - file: file.to_owned(), - jobs, - }) -} - -/// Lists the workflow file names inside an opened workflow directory. -/// -/// # Errors -/// -/// Returns an error when the directory cannot be listed or an entry's name -/// cannot be read. -fn workflow_names(dir: &Dir) -> Result, WorkflowError> { - let read = |err| WorkflowError::Read(WORKFLOW_DIR.to_owned(), err); - let mut names: Vec = Vec::new(); - for entry in dir.entries().map_err(read)? { - let name = entry.map_err(read)?.file_name().map_err(read)?; - let extension = Utf8Path::new(&name).extension().unwrap_or_default(); - if matches!(extension, "yml" | "yaml") { - names.push(name); - } - } - names.sort(); - Ok(names) -} - -/// Loads and parses every workflow beneath `root`. -/// -/// # Errors -/// -/// Returns an error when the directory cannot be opened or listed, a file -/// cannot be read, or a file is not a workflow document. -pub fn load_workflows_in(root: &Utf8Path) -> Result, WorkflowError> { - // The one ambient step: everything below reads through this capability, - // which cannot escape the workflow directory. - let dir = Dir::open_ambient_dir(root, ambient_authority()) - .map_err(|err| WorkflowError::Read(root.to_string(), err))?; - workflow_names(&dir)? - .iter() - .map(|name| { - let text = dir - .read_to_string(name) - .map_err(|err| WorkflowError::Read(name.clone(), err))?; - parse_workflow(name, &text) - }) - .collect() -} - -/// Loads and parses every workflow in this repository's `.github/workflows`. -/// -/// # Errors -/// -/// Returns the same errors as [`load_workflows_in`]. -pub fn load_workflows() -> Result, WorkflowError> { - let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(WORKFLOW_DIR); - load_workflows_in(&root) -} - -/// Reads a file from this repository's root through a directory capability. -/// -/// # Errors -/// -/// Returns an error when the repository root cannot be opened or the file -/// cannot be read. -pub fn read_repository_file(relative: &str) -> Result { - let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")); - let dir = Dir::open_ambient_dir(&root, ambient_authority()) - .map_err(|err| WorkflowError::Read(root.to_string(), err))?; - dir.read_to_string(relative) - .map_err(|err| WorkflowError::Read(relative.to_owned(), err)) -} - -/// Returns every step of every job, tagged with its workflow and job. -#[must_use] -pub fn all_steps(workflows: &[Workflow]) -> Vec<(String, String, Step)> { - workflows - .iter() - .flat_map(|workflow| { - workflow.jobs.iter().flat_map(move |job| { - job.steps - .iter() - .map(move |step| (workflow.file.clone(), job.id.clone(), step.clone())) - }) - }) - .collect() -} diff --git a/tests/workflow_contracts.rs b/tests/workflow_contracts.rs index 5109b65..5f8e4d5 100644 --- a/tests/workflow_contracts.rs +++ b/tests/workflow_contracts.rs @@ -13,15 +13,20 @@ #[path = "support/workflow_cache_owners.rs"] mod workflow_cache_owners; +#[path = "support/workflow_loader.rs"] +mod workflow_loader; #[path = "support/workflow_model.rs"] mod workflow_model; use camino::Utf8Path; use rstest::{fixture, rstest}; +use workflow_loader::{ + all_steps, load_workflows, load_workflows_in, parse_workflow, read_repository_file, +}; use workflow_model::{ - all_steps, load_workflows, load_workflows_in, parse_workflow, read_repository_file, Job, Step, - Workflow, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_SHA, UBICLOUD_LABEL, + Job, Step, Workflow, WorkflowSource, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_SHA, + UBICLOUD_LABEL, }; /// Fragments that mark a step as building a tool from source. @@ -346,7 +351,7 @@ fn the_uv_cache_names_its_layers_and_keys_them_by_runner(workflows: Vec Date: Fri, 4 Sep 2026 10:13:31 +0100 Subject: [PATCH 07/10] Make sccache actually reach Ubicloud's cache The shared setup action installs sccache but never exports the wrapper, so every compilation ran uncached. That was harmless while a `target` archive existed; with the archive gone, an unused compiler cache turns the change from a saving into a regression. Exporting the wrapper is necessary but not sufficient, and the two things it was missing are both silent failures. `SCCACHE_GHA_ENABLED` selects the GitHub Actions backend. Without it sccache reports `Local disk: ~/.cache/sccache`, which nothing persists between runs, and every request misses while the wrapper still costs its overhead. Chutoro measured 3,836 requests at a 0.18 % hit rate that way. The server binds its backend once, when it starts, and it must not start inside an action step. The Ubicloud runner re-injects `ACTIONS_CACHE_SERVICE_V2=on` and `ACTIONS_RESULTS_URL` into every `uses:` step, overriding whatever the credentials export wrote to `GITHUB_ENV`, so a server started by the shared action's `use-sccache: true` path binds GitHub's v2 service and its writes never reach Ubicloud's store. So both jobs now call `setup-rust` with `use-sccache: 'false'`, install a pinned sccache through `taiki-e/install-action` with `fallback: none`, and start it from a `run:` step after the export. A contract asserts the whole order: export, install, start, toolchain, build, report. Report to the log as well as the job summary. The summary cannot be read through the REST API, so statistics that went only there cannot be audited after the run. `Cache location` in the log is what distinguishes a working backend from a local directory nothing caches. Each job now also deletes `target/llvm-cov-target` once coverage exists, printing `df -h` either side. The tree has no later consumer, and a full disk has killed jobs silently, with no error text. `ci.yml` gains `workflow_dispatch`, with a contract, so a warm run can be measured without pushing a commit. Repin every shared action to c6125f19. The workflow model answers review findings on the support code. `runs-on` now parses all three shapes GitHub Actions accepts, not the scalar alone, so a label list or a runner group is no longer a spurious parse error; a step setting both `uses` and `run` is rejected, because the runner would accept neither reading. Action lookups compare the whole coordinate before the `@`, so `untrusted/setup-rust` can no longer satisfy a rule written about the shared one. A split cache is one owner only when exactly one restore and one save share its key: two restores, or a pair plus a third step, are separate owners, with properties for both. The model is split so the loading types and estate constants live in `workflow_estate.rs`, leaving the property tests a module of types they actually use. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- .github/workflows/ci.yml | 109 ++++++- .github/workflows/coverage-main.yml | 73 ++++- .github/workflows/dependabot-automerge.yml | 2 +- docs/developers-guide.md | 131 +++++++-- tests/support/workflow_assertions.rs | 76 +++++ tests/support/workflow_cache_owners.rs | 77 ++++- tests/support/workflow_estate.rs | 112 ++++++++ tests/support/workflow_loader.rs | 125 +++++++- tests/support/workflow_model.rs | 172 +++++------ tests/workflow_contracts.rs | 316 ++++++++++++++++----- tests/workflow_model_properties.rs | 45 ++- 11 files changed, 1006 insertions(+), 232 deletions(-) create mode 100644 tests/support/workflow_assertions.rs create mode 100644 tests/support/workflow_estate.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd5f153..bbfd195 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,6 +3,11 @@ name: CI on: pull_request: branches: [main] + # Warm-run dispatches. A dispatch restores the caches a pull request + # would restore and writes none of them: coverage-main.yml is the only + # writer on this repository, so a manual run can measure a warm build + # without displacing the trunk entry it read. + workflow_dispatch: jobs: build-test: @@ -18,6 +23,16 @@ jobs: # Bevy's render features make the coverage build heavy; lift the # shared-action cargo wall-clock cap (default 600 s) accordingly. RUN_RUST_CARGO_WAIT_TIMEOUT: '1800' + # sccache is the sole owner of compiler output: no step archives a + # `target` tree. RUSTC_WRAPPER is what engages it, and + # SCCACHE_GHA_ENABLED is what points it at the Actions cache. Without + # the second, sccache falls back to `~/.cache/sccache`, which nothing + # persists, and the wrapper becomes pure overhead. + RUSTC_WRAPPER: sccache + SCCACHE_GHA_ENABLED: 'true' + # sccache cannot cache incremental compilation, and an incremental + # build would defeat every hit. + CARGO_INCREMENTAL: '0' steps: - uses: actions/checkout@v7 with: @@ -25,17 +40,67 @@ jobs: # (`upload-codescene-coverage` with `mode: check`) can reach # the pull request's merge base. fetch-depth: 0 + - name: Route the compiler cache into Ubicloud's store + # sccache's GitHub Actions backend reads these from the environment, + # but the runner exposes them to action code rather than to later + # steps, so re-export them for the shell steps that compile. On + # Ubicloud `ACTIONS_CACHE_URL` names the runner's local cache proxy, + # which is what puts sccache's traffic in Ubicloud's store instead of + # GitHub's. `ACTIONS_CACHE_SERVICE_V2` is cleared because the v2 + # service bypasses that proxy; exporting `ACTIONS_RESULTS_URL` does + # not route through it either (measured 2026-09-04). + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + with: + script: | + // Report where the endpoint came from, and whether a token was + // present at all, so a misconfigured backend is diagnosable from + // the log rather than from a slower build. Never print the token. + const cacheUrl = process.env.ACTIONS_CACHE_URL ?? ''; + const runtimeToken = process.env.ACTIONS_RUNTIME_TOKEN ?? ''; + core.info(`sccache cache endpoint present: ${Boolean(cacheUrl)}`); + core.info(`sccache runtime token present: ${Boolean(runtimeToken)}`); + if (!cacheUrl || !runtimeToken) { + core.warning( + 'sccache has no Actions cache endpoint; every compilation ' + + 'will miss and the wrapper will only add overhead', + ); + } + if (runtimeToken) { + core.setSecret(runtimeToken); + } + core.exportVariable('ACTIONS_CACHE_URL', cacheUrl); + core.exportVariable('ACTIONS_RUNTIME_TOKEN', runtimeToken); + core.exportVariable('ACTIONS_CACHE_SERVICE_V2', ''); + - name: Install sccache + # A pinned prebuilt binary, with `fallback: none` so the action fails + # rather than compiling sccache from source. Installing inside an + # action step is safe; starting the server there is not, which is why + # the next step is a `run:` step. + uses: taiki-e/install-action@c7eb1735f09259a5035e8e5d44b1406b1cddc0fb # v2.83.0 + with: + tool: sccache@0.16.0 + fallback: none + - name: Reset compiler-cache counters + # This starts the sccache server, and it must be a `run:` step. The + # Ubicloud runner re-injects `ACTIONS_CACHE_SERVICE_V2=on` and + # `ACTIONS_RESULTS_URL` into every action step, overriding what the + # export above wrote to `GITHUB_ENV`, so a server started inside an + # action binds GitHub's v2 service and its writes never reach + # Ubicloud's store. A `run:` step sees only `GITHUB_ENV`. + run: | + set -euo pipefail + sccache --version + sccache --zero-stats - name: Setup Rust - uses: leynos/shared-actions/.github/actions/setup-rust@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/actions/setup-rust@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: # Sole owner of ~/.cargo/registry and ~/.cargo/git for this job. It # runs before the first cargo invocation so the lint step reads a # warm registry. cache-provider: github - # No RUSTC_WRAPPER is configured anywhere in this repository, so an - # sccache install would download a binary that never serves a - # compilation and would have no cache owner. Adopting sccache is a - # separate, measured change. + # The job installs and starts sccache itself, above. Letting this + # action do it would start the server inside an action step, where + # the runner's re-injected variables bind GitHub's v2 service. use-sccache: 'false' - name: Cache uv tool layers # `make spelling` drives uv with repository-local UV_CACHE_DIR and @@ -59,7 +124,7 @@ jobs: # Downloads the pinned prebuilt installer and verifies it against a # digest pinned in the action. The action owns the cache for the # installer binary, its version marker, and ~/.local/share/whitaker. - uses: leynos/shared-actions/.github/actions/install-whitaker@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/actions/install-whitaker@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: installer-version: '0.2.7' cache-provider: github @@ -72,7 +137,7 @@ jobs: # evidence and doubled the billed compile. `all-features` names exactly # the set the explicit feature list used to name. - name: Generate coverage - uses: leynos/shared-actions/.github/actions/generate-coverage@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/actions/generate-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: all-features: 'true' all-targets: 'true' @@ -84,6 +149,34 @@ jobs: # `setup-rust` above already owns ~/.cargo/registry and ~/.cargo/git; # this keeps the action from becoming a second owner of them. cache-provider: external + - name: Reclaim the coverage scratch tree + # The instrumented tree has no later consumer. Deleting it before the + # caches are saved keeps a disk that has silently killed jobs on + # smaller shapes from filling, and the two `df` calls make a shrinking + # margin visible before it becomes a failure. + if: always() + run: | + set -euo pipefail + df -h . + rm -rf -- target/llvm-cov-target + df -h . + - name: Record compiler-cache effectiveness + if: always() + shell: bash + run: | + set -euo pipefail + # Print to the log as well as the job summary: the summary is not + # readable through the REST API, so the log copy is what lets anyone + # confirm `Cache location`, the hit rate, and any read or write + # error after the fact. + stats="$(sccache --show-stats)" + printf '%s\n' "$stats" + { + printf '### sccache statistics (%s)\n\n' "${GITHUB_JOB}" + printf '```text\n' + printf '%s\n' "$stats" + printf '```\n' + } >> "${GITHUB_STEP_SUMMARY}" # The CodeScene changed-line gate: diffs the PR against its merge # base and evaluates changed-line coverage. Guarded so secret-less # runs (forks, repos not yet onboarded) skip rather than fail. @@ -92,7 +185,7 @@ jobs: env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} if: env.CS_ACCESS_TOKEN != '' && github.event_name == 'pull_request' - uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: format: lcov mode: check diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index 9ff87f0..580b4c5 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -26,19 +26,58 @@ jobs: # Bevy's render features make the coverage build heavy; lift the # shared-action cargo wall-clock cap (default 600 s) accordingly. RUN_RUST_CARGO_WAIT_TIMEOUT: '1800' + # See ci.yml. This job is the trunk writer, so it is the run that + # populates the compiler cache every pull request then reads. + RUSTC_WRAPPER: sccache + SCCACHE_GHA_ENABLED: 'true' + CARGO_INCREMENTAL: '0' steps: - uses: actions/checkout@v7 + - name: Route the compiler cache into Ubicloud's store + # See ci.yml for why each variable is exported, and why this must + # precede the step that starts the sccache server. + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + with: + script: | + const cacheUrl = process.env.ACTIONS_CACHE_URL ?? ''; + const runtimeToken = process.env.ACTIONS_RUNTIME_TOKEN ?? ''; + core.info(`sccache cache endpoint present: ${Boolean(cacheUrl)}`); + core.info(`sccache runtime token present: ${Boolean(runtimeToken)}`); + if (!cacheUrl || !runtimeToken) { + core.warning( + 'sccache has no Actions cache endpoint; every compilation ' + + 'will miss and the wrapper will only add overhead', + ); + } + if (runtimeToken) { + core.setSecret(runtimeToken); + } + core.exportVariable('ACTIONS_CACHE_URL', cacheUrl); + core.exportVariable('ACTIONS_RUNTIME_TOKEN', runtimeToken); + core.exportVariable('ACTIONS_CACHE_SERVICE_V2', ''); + - name: Install sccache + uses: taiki-e/install-action@c7eb1735f09259a5035e8e5d44b1406b1cddc0fb # v2.83.0 + with: + tool: sccache@0.16.0 + fallback: none + - name: Reset compiler-cache counters + # A `run:` step, for the reason given in ci.yml: a server started + # inside an action step binds GitHub's v2 service instead of the + # Ubicloud proxy. + run: | + set -euo pipefail + sccache --version + sccache --zero-stats - name: Setup Rust - uses: leynos/shared-actions/.github/actions/setup-rust@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/actions/setup-rust@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: # Sole owner of ~/.cargo/registry and ~/.cargo/git for this job, and # the trunk writer whose entry every pull-request run restores. cache-provider: github - # See ci.yml: nothing sets RUSTC_WRAPPER, so the sccache install - # would download a binary that serves no compilation. + # The job installs and starts sccache itself, above. use-sccache: 'false' - name: Generate coverage - uses: leynos/shared-actions/.github/actions/generate-coverage@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/actions/generate-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: all-features: 'true' all-targets: 'true' @@ -49,11 +88,35 @@ jobs: with-ratchet: 'true' # `setup-rust` above already owns ~/.cargo/registry and ~/.cargo/git. cache-provider: external + - name: Reclaim the coverage scratch tree + # See ci.yml. This job saves the trunk cache entries, so the scratch + # tree must be gone before the saves rather than after them. + if: always() + run: | + set -euo pipefail + df -h . + rm -rf -- target/llvm-cov-target + df -h . + - name: Record compiler-cache effectiveness + if: always() + shell: bash + run: | + set -euo pipefail + # To the log as well as the summary: the summary is not readable + # through the REST API. + stats="$(sccache --show-stats)" + printf '%s\n' "$stats" + { + printf '### sccache statistics (%s)\n\n' "${GITHUB_JOB}" + printf '```text\n' + printf '%s\n' "$stats" + printf '```\n' + } >> "${GITHUB_STEP_SUMMARY}" - name: Upload coverage data to CodeScene env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} if: env.CS_ACCESS_TOKEN != '' - uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: format: lcov access-token: ${{ env.CS_ACCESS_TOKEN }} diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml index 9f85c06..90d1aa1 100644 --- a/.github/workflows/dependabot-automerge.yml +++ b/.github/workflows/dependabot-automerge.yml @@ -28,6 +28,6 @@ jobs: # The token is not used for any external cloud auth. id-token: write if: ${{ github.event_name == 'workflow_dispatch' || github.actor == 'dependabot[bot]' }} - uses: leynos/shared-actions/.github/workflows/dependabot-automerge.yml@7d46a399558914f5a05074e55a560fec0269fd0d + uses: leynos/shared-actions/.github/workflows/dependabot-automerge.yml@c6125f19593668cbfefd65a59c08cb7aefe90d93 with: pull-request-number: ${{ inputs.pull-request-number || github.event.pull_request.number }} diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e0d55fd..93eb773 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -340,7 +340,6 @@ buffered-message compile-pass harness `cargo clippy --all-targets --all-features -- -D warnings`, and the Whitaker Dylint suite. - ## Continuous integration Two workflows do the developer-blocking work. `ci.yml`'s `build-test` job runs @@ -355,7 +354,6 @@ release orchestration are API-bound, so paid runner capacity buys them nothing and their queue time is already short. `dependabot-automerge.yml` calls a reusable workflow, which chooses its own runner. - ### Tool installation No tool is compiled from source. `whitaker-installer` is installed by @@ -363,8 +361,12 @@ No tool is compiled from source. `whitaker-installer` is installed by pinned prebuilt release archive and verifies it against a digest pinned inside the action, then runs the installer to place the Whitaker Dylint suite. Every `leynos/shared-actions` reference pins commit -`7d46a399558914f5a05074e55a560fec0269fd0d`. +`c6125f19593668cbfefd65a59c08cb7aefe90d93`. +sccache is installed the same way, by `taiki-e/install-action` with +`tool: sccache@0.16.0` and `fallback: none`. The fallback matters: without it +the action would compile sccache from source when no prebuilt binary matched, +which is the outcome the rule exists to prevent. ### Cache ownership @@ -377,6 +379,7 @@ every miss is explainable from the rendered key. | `~/.cargo/bin/whitaker-installer`, its version marker, `~/.local/share/whitaker` | `install-whitaker` (`cache-provider: github`) | `runner.os`, `runner.arch`, installer version, `dylint.toml` hash | | `.uv-cache`, `.uv-tools` | the `Cache uv tool layers` step in `ci.yml` | `runner.os`, `runner.arch`, `runner.environment`, `Makefile` and `scripts/*.py` hash | | coverage ratchet baseline files | `generate-coverage`'s split restore and save | `runner.os`, run id | +| compiler output | `sccache`, through its GitHub Actions backend | compiler flags and toolchain, hashed by sccache itself | *Table 1: Cache ownership and cache-key inputs.* @@ -389,17 +392,71 @@ reference written in these workflow files pins workflow files only. A shared action may reach an `actions/cache` reference of its own, and `upload-codescene-coverage` does; the next paragraph records it. -`use-sccache: 'false'` is passed to `setup-rust` because nothing in this -repository sets `RUSTC_WRAPPER`. Installing sccache would download a binary -that serves no compilation and has no cache owner. Adopting a compiler cache is -a separate, measured change. - -Two downloads remain deliberately uncached. The `cs-coverage` CLI is fetched on +### The compiler cache + +sccache owns compiler output, and nothing archives a `target` tree. Both build +jobs wire it up as four steps in a fixed order, and the order is the whole +point: get it wrong and the cache is silently a no-op. + +The job sets `RUSTC_WRAPPER: sccache` and `SCCACHE_GHA_ENABLED: 'true'` at job +level. The first engages the wrapper; the second is what selects the GitHub +Actions backend. Without the second, sccache falls back to +`Local disk: ~/.cache/sccache`, which nothing persists between runs, so the +wrapper becomes pure overhead. `CARGO_INCREMENTAL: '0'` accompanies them +because sccache cannot cache an incremental compilation. + +**Export.** A pinned `actions/github-script` step, after checkout, re-exports +`ACTIONS_CACHE_URL` and `ACTIONS_RUNTIME_TOKEN` into `GITHUB_ENV` and clears +`ACTIONS_CACHE_SERVICE_V2`. The runner gives those two variables to action code +but not to later shell steps, and sccache's backend reads them from the +environment. On Ubicloud `ACTIONS_CACHE_URL` names the runner's local cache +proxy, so re-exporting it is what puts the compiler cache in Ubicloud's store +rather than GitHub's. The v2 cache service bypasses that proxy, so it is +cleared; exporting `ACTIONS_RESULTS_URL` does not route through the proxy +either. The step also logs whether an endpoint and a token were present, and +warns when either is missing, so a misconfigured backend is diagnosable from +the log rather than from an unexplained slow build. It never prints the token. + +**Install.** `taiki-e/install-action` places the pinned sccache binary. An +action step is safe here because installing a binary does not start the sccache +server. + +**Start.** A `run:` step runs `sccache --zero-stats`, which starts the server. +This must be a `run:` step and it must follow the export. The Ubicloud runner +re-injects `ACTIONS_CACHE_SERVICE_V2=on` and `ACTIONS_RESULTS_URL` into every +action step, overriding what the export wrote to `GITHUB_ENV`. A server started +inside an action step, which is what `setup-rust` with `use-sccache: 'true'` +would do, therefore binds GitHub's v2 service; its writes then fail and nothing +reaches Ubicloud's store. The server binds its backend once, at start, so a +later change to the environment is invisible to it. That is why `setup-rust` is +called with `use-sccache: 'false'` in both jobs. + +**Report.** `sccache --show-stats` runs after the build, printing the counters +to the log as well as to the job summary. The log copy is the one that matters: +the summary cannot be read through the REST API, so it cannot be checked after +the fact. Read `Cache location` on every run. It must name the GitHub Actions +backend; `Local disk: ~/.cache/sccache` means the backend was never selected +and nothing is being cached. A warm build reporting zero hits is a broken +contract, not a slow one. + +Each job also deletes `target/llvm-cov-target` once coverage has been +generated, printing `df -h` either side. The instrumented tree has no later +consumer, and on smaller runner shapes a full disk has killed a job silently, +with no error text. + +`ci.yml` accepts `workflow_dispatch` so a warm run can be measured on demand. +A dispatch restores what a pull request restores and writes nothing: +`coverage-main.yml` is the only job that saves on this repository. + +One download remains deliberately uncached. The `cs-coverage` CLI is fetched on every run because `upload-codescene-coverage` only caches it when `cli-version` is pinned, and its cache step uses an unpinned `actions/cache@v4` that this -repository cannot pin from here. The uv tool layers are cached by the -pull-request job that installs them, which is also the only job that installs -them; there is no trunk job to designate as the sole writer instead. +repository cannot pin from here. + +The uv tool layers are an exception to the trunk-writer rule rather than to the +ownership rule. They are cached by the pull-request job that installs them, +which is also the only job that installs them, so there is no trunk job to +designate as the sole writer instead. ### One test execution per pull request @@ -409,7 +466,6 @@ former separate `cargo test` step covered and more, for one compile rather than two. A workflow contract in `tests/workflow_contracts.rs` fails if a second `cargo test` or `cargo nextest` step reappears in either job. - ### Workflow contracts `tests/workflow_contracts.rs` asserts the rules @@ -419,16 +475,32 @@ runner labels, an installer before the first use of what it installs, and a single test execution per build job. It also pins the inputs that make those rules true, so a workflow cannot keep the shape of the policy while dropping its substance: `cache-provider`, `use-sccache`, the Whitaker installer -version, the coverage flags, the uv cache paths and key, and a bounded -`timeout-minutes` for each build job. - -`tests/support/workflow_model.rs` holds the types and the queries the -contracts ask of them, and `tests/support/workflow_loader.rs` turns files into -those values. Parsing is strict: a workflow field that is present but of the -wrong type is an error rather than a silent default, because a contract that -read an empty string for a mistyped `runs-on` would pass a workflow it should -reject. The files are read through a `cap_std` directory capability rooted at -`.github/workflows`. +version, the coverage flags, the uv cache paths and key, a bounded +`timeout-minutes` for each build job, and the compiler-cache wiring: the two +job-level variables, and the export, install, start, build, report order that +the sccache server's one-shot backend binding depends on. + +`tests/support/workflow_model.rs` holds the job, step, and runner-selection +types the properties and the contracts share; +`tests/support/workflow_estate.rs` holds the pinned commits, the whole-file +`Workflow` type, and the errors parsing reports, which only the contracts need. +`tests/support/workflow_loader.rs` turns files into those values. + +Parsing is strict about shape and permissive about spelling. A field that is +present but of the wrong type is an error rather than a silent default, because +a contract that read an empty string for a mistyped `runs-on` would pass a +workflow it should reject. A step that sets both `uses` and `run` is rejected +too: GitHub Actions runs a step one way or the other, never both. Against that, +every form the platform genuinely accepts must parse. `runs-on` may be a label, +a list of labels, or a mapping naming a runner group, and `on` may be an event, +a list, or a mapping, read under the bare key that YAML 1.1 turns into the +boolean true. The files are read through a `cap_std` directory capability +rooted at `.github/workflows`. + +Action references are matched on the whole coordinate before the `@`, publisher +included. A suffix match would let `untrusted/setup-rust` satisfy a rule +written about the shared `setup-rust`, which is the opposite of what a pinning +rule is for. Two assurance methods are used together, following [ADR 003](adr-003-bounded-rstest-over-property-testing.md). @@ -436,7 +508,12 @@ Two assurance methods are used together, following files as they stand, and `tests/workflow_model_properties.rs` samples the wider domain with `proptest`: arbitrary step orderings, repeated display names, interleaved unrelated steps, and split caches whose halves agree or -disagree on a key. The properties check cache-owner uniqueness and -installer-ordering against small oracles written independently of the -implementation. Run both with `make test`, and run `actionlint` after editing -any workflow. +disagree on a key, or where a third step claims a paired key. The properties +check cache-owner uniqueness and installer-ordering against small oracles +written independently of the implementation. Run both with `make test`, and run +`actionlint` after editing any workflow. + +Only one restore and one save sharing a key count as a single owner. Two +restores on the same key are two owners, and so are a matching pair plus a +third step, because otherwise a genuine duplicate could hide behind the +split-cache exception. diff --git a/tests/support/workflow_assertions.rs b/tests/support/workflow_assertions.rs new file mode 100644 index 0000000..cdc095f --- /dev/null +++ b/tests/support/workflow_assertions.rs @@ -0,0 +1,76 @@ +//! Shared fixtures and assertion helpers for the workflow contracts. +//! +//! The contracts read the same estate and ask the same three questions of it: +//! give me that job, give me the step that uses that action, and tell me an +//! input matches. Keeping those here leaves each contract file holding only +//! the rules it asserts. +//! +//! # Examples +//! +//! ```no_run +//! let estate = workflow_assertions::workflows(); +//! let job = workflow_assertions::job_named(&estate, "build-test"); +//! assert_eq!(job.runs_on.labels(), ["ubicloud-standard-8"]); +//! ``` + +use rstest::fixture; + +use crate::workflow_estate::Workflow; +use crate::workflow_loader::load_workflows; +use crate::workflow_model::{Job, Step}; + +/// Every workflow in `.github/workflows`, parsed once per test. +#[fixture] +pub fn workflows() -> Vec { + match load_workflows() { + Ok(estate) => estate, + Err(err) => panic!("workflow estate must parse: {err}"), + } +} + +/// Returns every job in the estate, tagged with its workflow file. +pub fn jobs(workflows: &[Workflow]) -> Vec<(String, Job)> { + workflows + .iter() + .flat_map(|workflow| { + workflow + .jobs + .iter() + .map(move |job| (workflow.file.clone(), job.clone())) + }) + .collect() +} + +/// Returns the job with the given id, or panics naming the missing job. +pub fn job_named<'a>(workflows: &'a [Workflow], id: &str) -> &'a Job { + let found = workflows + .iter() + .flat_map(|workflow| workflow.jobs.iter()) + .find(|job| job.id == id); + let Some(job) = found else { + panic!("workflow estate must define the `{id}` job") + }; + job +} + +/// Returns a job's step that uses `coordinate`, or panics naming both. +/// +/// `coordinate` is the whole action reference before the `@`, publisher +/// included, so a same-named action from another publisher cannot answer for +/// the one the contract meant. +pub fn step_using<'a>(job: &'a Job, coordinate: &str) -> &'a Step { + let Some(step) = job.step_using(coordinate) else { + panic!("`{}` must use the `{coordinate}` action", job.id) + }; + step +} + +/// Asserts that a step supplies the expected value for one input. +pub fn assert_input(job_id: &str, step: &Step, key: &str, expected: &str) { + assert_eq!( + step.input(key), + expected, + "`{job_id}` step `{}` must set `{key}: {expected}`", + step.label() + ); +} diff --git a/tests/support/workflow_cache_owners.rs b/tests/support/workflow_cache_owners.rs index 45dcb26..0218d1e 100644 --- a/tests/support/workflow_cache_owners.rs +++ b/tests/support/workflow_cache_owners.rs @@ -19,7 +19,7 @@ //! assert!(owners.iter().all(|owner| !owner.path.is_empty())); //! ``` -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use crate::workflow_model::{Job, Step}; @@ -35,7 +35,7 @@ pub struct CacheOwner { /// Paths a shared composite action caches when `cache-provider` is `github`. /// /// These mirror the action definitions at -/// `leynos/shared-actions@7d46a399558914f5a05074e55a560fec0269fd0d`. A caller +/// `leynos/shared-actions@c6125f19593668cbfefd65a59c08cb7aefe90d93`. A caller /// that sets `cache-provider: external` takes the path away from the action, /// which is how a second owner of the Cargo registry is avoided. const SHARED_ACTION_CACHES: [(&str, &[&str]); 3] = [ @@ -71,31 +71,54 @@ fn action_path(uses: &str) -> &str { uses.split('@').next().unwrap_or_default() } +/// The half of a split cache a step is, if it is one. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SplitHalf { + /// An `actions/cache/restore` step. + Restore, + /// An `actions/cache/save` step. + Save, +} + +/// Returns which half of a split cache a step is, or `None` for other steps. +fn split_half(uses: &str) -> Option { + match action_path(uses) { + "actions/cache/restore" => Some(SplitHalf::Restore), + "actions/cache/save" => Some(SplitHalf::Save), + _ => None, + } +} + /// Identity of the owner making a claim. /// /// A whole-cache or shared-action claim is owned by its step alone. A split -/// cache is owned jointly by the restore and save steps that share its key, -/// so both halves report the same identity and are not counted twice. -fn owner_identity(step: &Step, index: usize) -> String { +/// cache is owned jointly by the one restore and the one save step that share +/// its key, so those two halves report the same identity and are not counted +/// twice. +/// +/// `paired` says whether this step is half of exactly such a pair. Two +/// restores sharing a key are two owners, not one, and so are a restore, a +/// save, and a second restore: only a step with exactly one counterpart of the +/// other half may share an identity with it. Without that condition a +/// duplicate claim could hide behind the split-cache exception. +fn owner_identity(step: &Step, index: usize, paired: bool) -> String { let label = if step.name.is_empty() { step.uses.as_str() } else { step.name.as_str() }; - match action_path(&step.uses) { - "actions/cache/restore" | "actions/cache/save" => { - format!("split cache with key `{}`", step.input("key")) - } - _ => format!("step {index} (`{label}`)"), + if paired { + return format!("split cache with key `{}`", step.input("key")); } + format!("step {index} (`{label}`)") } /// Returns the claims an `actions/cache` step makes on its own `path` input. -fn direct_owners(step: &Step, index: usize) -> Vec { +fn direct_owners(step: &Step, index: usize, paired: bool) -> Vec { if !action_path(&step.uses).starts_with("actions/cache") { return Vec::new(); } - let owner = owner_identity(step, index); + let owner = owner_identity(step, index, paired); step.cache_paths() .into_iter() .map(|path| CacheOwner { @@ -116,7 +139,7 @@ fn shared_owners(step: &Step, index: usize) -> Vec { if !provider.is_empty() && provider != "github" { return Vec::new(); } - let owner = owner_identity(step, index); + let owner = owner_identity(step, index, false); SHARED_ACTION_CACHES .iter() .filter(|(action, _)| *action == name) @@ -128,14 +151,40 @@ fn shared_owners(step: &Step, index: usize) -> Vec { .collect() } +/// Returns the keys for which exactly one restore step and one save step exist. +/// +/// Only those keys join their two steps into a single owner. A key claimed by +/// two restores, or by a pair plus a third step, leaves every one of its steps +/// an owner in its own right, which is what makes the duplicate visible. +fn paired_split_keys(job: &Job) -> BTreeSet { + let mut halves: BTreeMap = BTreeMap::new(); + for step in &job.steps { + let Some(half) = split_half(&step.uses) else { + continue; + }; + let counts = halves.entry(step.input("key").to_owned()).or_default(); + match half { + SplitHalf::Restore => counts.0 += 1, + SplitHalf::Save => counts.1 += 1, + } + } + halves + .into_iter() + .filter(|(_, (restores, saves))| *restores == 1 && *saves == 1) + .map(|(key, _)| key) + .collect() +} + /// Returns every cache claim made by a job, in step order. #[must_use] pub fn owners_for(job: &Job) -> Vec { + let paired = paired_split_keys(job); job.steps .iter() .enumerate() .flat_map(|(index, step)| { - let mut claims = direct_owners(step, index); + let is_paired = split_half(&step.uses).is_some() && paired.contains(step.input("key")); + let mut claims = direct_owners(step, index, is_paired); claims.extend(shared_owners(step, index)); claims }) diff --git a/tests/support/workflow_estate.rs b/tests/support/workflow_estate.rs new file mode 100644 index 0000000..ed9dc7e --- /dev/null +++ b/tests/support/workflow_estate.rs @@ -0,0 +1,112 @@ +//! Loading-facing and contract-facing workflow support. +//! +//! The estate's pinned commits and runner labels, the errors and locations +//! parsing reports, and the whole-file `Workflow` type. `workflow_model.rs` +//! holds the job and step shapes these are built from, which the property +//! tests share. +//! +//! # Examples +//! +//! ```no_run +//! let at = workflow_estate::Location::file("ci.yml"); +//! assert!(at.shape("bad").to_string().contains("ci.yml")); +//! ``` + +use std::fmt; + +use crate::workflow_model::Job; + +/// Directory holding the repository's workflow definitions. +pub const WORKFLOW_DIR: &str = ".github/workflows"; + +/// Commit that every `actions/cache` reference must pin (v6.1.0). +pub const CACHE_ACTION_SHA: &str = "55cc8345863c7cc4c66a329aec7e433d2d1c52a9"; + +/// Commit that every `leynos/shared-actions` reference must pin. +pub const SHARED_ACTIONS_SHA: &str = "c6125f19593668cbfefd65a59c08cb7aefe90d93"; + +/// Runner label used by this repository's Ubicloud build and test jobs. +pub const UBICLOUD_LABEL: &str = "ubicloud-standard-8"; + +/// Publisher whose composite actions this repository is allowed to call. +pub const SHARED_ACTIONS_OWNER: &str = "leynos/shared-actions"; + +/// Jobs that build or test the crate and therefore keep an Ubicloud label. +pub const BUILD_JOB_IDS: [&str; 2] = ["build-test", "coverage-upload"]; + +/// Failure encountered while reading or parsing the workflow estate. +#[derive(Debug)] +pub enum WorkflowError { + /// A workflow file or the workflow directory could not be read. + Read(String, std::io::Error), + /// A workflow file was not valid YAML. + Parse(String, serde_norway::Error), + /// A workflow file was structurally unusable. + Shape(String, String), +} + +impl fmt::Display for WorkflowError { + /// Renders the failure with the workflow name that produced it. + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Read(name, err) => write!(f, "cannot read {name}: {err}"), + Self::Parse(name, err) => write!(f, "cannot parse {name}: {err}"), + Self::Shape(name, msg) => write!(f, "unexpected shape in {name}: {msg}"), + } + } +} + +impl std::error::Error for WorkflowError {} + +/// Where in the estate a value was read, carried instead of a bare string so +/// the parsing helpers take one string argument rather than several. +#[derive(Debug, Clone)] +pub struct Location(String); + +impl Location { + /// Locates a whole workflow file. + #[must_use] + pub fn file(name: &str) -> Self { + Self(name.to_owned()) + } + + /// Locates one job within this file. + #[must_use] + pub fn job(&self, id: &str) -> Self { + Self(format!("{}: job `{id}`", self.0)) + } + + /// Builds a shape error reported at this location. + #[must_use] + pub fn shape(&self, message: &str) -> WorkflowError { + WorkflowError::Shape(self.0.clone(), message.to_owned()) + } +} + +/// A workflow document paired with the file name it came from. +#[derive(Debug, Clone, Copy)] +pub struct WorkflowSource<'a> { + /// File name within [`WORKFLOW_DIR`]. + pub file: &'a str, + /// The document's YAML text. + pub text: &'a str, +} + +/// One workflow file. +#[derive(Debug, Clone)] +pub struct Workflow { + /// File name within [`WORKFLOW_DIR`]. + pub file: String, + /// Event names under `on`, in declaration order. + pub triggers: Vec, + /// Jobs in declaration order. + pub jobs: Vec, +} + +impl Workflow { + /// Reports whether the workflow declares the named trigger. + #[must_use] + pub fn has_trigger(&self, event: &str) -> bool { + self.triggers.iter().any(|name| name == event) + } +} diff --git a/tests/support/workflow_loader.rs b/tests/support/workflow_loader.rs index a74f285..d7091f5 100644 --- a/tests/support/workflow_loader.rs +++ b/tests/support/workflow_loader.rs @@ -14,7 +14,7 @@ //! ```no_run //! let workflows = workflow_loader::load_workflows()?; //! assert!(workflows.iter().any(|w| w.file == "ci.yml")); -//! # Ok::<(), workflow_model::WorkflowError>(()) +//! # Ok::<(), workflow_estate::WorkflowError>(()) //! ``` use std::collections::BTreeMap; @@ -23,9 +23,8 @@ use camino::{Utf8Path, Utf8PathBuf}; use cap_std::{ambient_authority, fs_utf8::Dir}; use serde_norway::Value; -use crate::workflow_model::{ - Job, Location, Step, Workflow, WorkflowError, WorkflowSource, WORKFLOW_DIR, -}; +use crate::workflow_estate::{Location, Workflow, WorkflowError, WorkflowSource, WORKFLOW_DIR}; +use crate::workflow_model::{Job, RunnerSelection, Step}; /// Renders a YAML scalar as the string a workflow expression would see. /// @@ -70,18 +69,32 @@ fn optional_u64(raw: &Value, key: &str, at: &Location) -> Result, Wo /// Returns an error when `with` is not a mapping or an input is not a scalar. fn parse_inputs(raw: &Value, at: &Location) -> Result, WorkflowError> { - let Some(value) = raw.get("with") else { + parse_scalar_mapping(raw, "with", at) +} + +/// Parses an optional mapping of scalars, such as `with` or `env`. +/// +/// # Errors +/// +/// Returns an error when the field is not a mapping or a value is not a +/// scalar. +fn parse_scalar_mapping( + raw: &Value, + field: &str, + at: &Location, +) -> Result, WorkflowError> { + let Some(value) = raw.get(field) else { return Ok(BTreeMap::new()); }; let mapping = value .as_mapping() - .ok_or_else(|| at.shape("`with` must be a mapping"))?; + .ok_or_else(|| at.shape(&format!("`{field}` must be a mapping")))?; mapping .iter() .map(|(key, item)| { let name = key .as_str() - .ok_or_else(|| at.shape("every `with` key must be a string"))?; + .ok_or_else(|| at.shape(&format!("every `{field}` key must be a string")))?; let rendered = render_scalar(item) .ok_or_else(|| at.shape(&format!("input `{name}` must be a scalar")))?; Ok((name.to_owned(), rendered)) @@ -105,10 +118,58 @@ fn parse_step(raw: &Value, at: &Location) -> Result { run: optional_string(raw, "run", at)?, with: parse_inputs(raw, at)?, }; - if step.uses.is_empty() && step.run.is_empty() { - return Err(at.shape("every step must set `uses` or `run`")); + match (step.uses.is_empty(), step.run.is_empty()) { + (true, true) => Err(at.shape("every step must set `uses` or `run`")), + // GitHub Actions rejects a step that both calls an action and runs a + // script, so accepting one here would let the contracts reason about a + // step shape the runner would never execute. + (false, false) => Err(at.shape("a step must not set both `uses` and `run`")), + _ => Ok(step), + } +} + +/// Reads a sequence of label strings from a `runs-on` value. +/// +/// # Errors +/// +/// Returns an error when an entry is not a scalar. +fn parse_labels(value: &Value, at: &Location) -> Result, WorkflowError> { + match value { + Value::Sequence(items) => items + .iter() + .map(|item| { + render_scalar(item) + .ok_or_else(|| at.shape("every `runs-on` label must be a scalar")) + }) + .collect(), + _ => render_scalar(value) + .map(|label| vec![label]) + .ok_or_else(|| at.shape("`runs-on` must be a label, a list of labels, or a mapping")), + } +} + +/// Parses a job's `runs-on` in any of the three shapes GitHub Actions accepts. +/// +/// # Errors +/// +/// Returns an error when the value is a mapping without a `group`, or when a +/// label is not a scalar. +fn parse_runs_on(raw: &Value, at: &Location) -> Result { + let Some(value) = raw.get("runs-on") else { + return Ok(RunnerSelection::Delegated); + }; + if value.as_mapping().is_none() { + return Ok(RunnerSelection::Labels(parse_labels(value, at)?)); } - Ok(step) + let group = value + .get("group") + .and_then(render_scalar) + .ok_or_else(|| at.shape("a mapping `runs-on` must name a `group`"))?; + let labels = match value.get("labels") { + None => Vec::new(), + Some(labels) => parse_labels(labels, at)?, + }; + Ok(RunnerSelection::Group { group, labels }) } /// Parses one job of a workflow. @@ -130,17 +191,56 @@ fn parse_job(id: &str, raw: &Value, file: &Location) -> Result Result, WorkflowError> { + let raw = document + .get("on") + .or_else(|| document.get(Value::Bool(true))) + .ok_or_else(|| at.shape("missing an `on` trigger"))?; + if let Some(mapping) = raw.as_mapping() { + return mapping + .keys() + .map(|key| { + key.as_str() + .map(ToOwned::to_owned) + .ok_or_else(|| at.shape("every `on` key must be a string")) + }) + .collect(); + } + if let Some(items) = raw.as_sequence() { + return items + .iter() + .map(|item| { + render_scalar(item).ok_or_else(|| at.shape("every `on` entry must be a scalar")) + }) + .collect(); + } + render_scalar(raw) + .map(|event| vec![event]) + .ok_or_else(|| at.shape("`on` must be an event, a list of events, or a mapping")) +} + /// Parses one workflow document. /// /// # Errors @@ -166,6 +266,7 @@ pub fn parse_workflow(source: WorkflowSource<'_>) -> Result, WorkflowError>>()?; Ok(Workflow { file: source.file.to_owned(), + triggers: parse_triggers(&document, &at)?, jobs, }) } diff --git a/tests/support/workflow_model.rs b/tests/support/workflow_model.rs index c81d449..fe2e4f4 100644 --- a/tests/support/workflow_model.rs +++ b/tests/support/workflow_model.rs @@ -1,91 +1,72 @@ -//! Types describing the repository's GitHub Actions workflow estate. +//! The workflow shapes the property tests and the contracts both reason about. //! -//! The workflow-contract tests assert placement, tool-install, and cache -//! ownership rules over these types rather than over raw YAML text, so a -//! reordered key or a reflowed block scalar cannot silently defeat a rule. -//! `workflow_loader` turns files into these values; this module holds only -//! the shapes and the queries the contracts ask of them. +//! A job, its steps, and how it selects a runner. Everything needed to load +//! workflows from disk, and everything only the contracts ask for, lives in +//! `workflow_estate.rs` instead, so a test binary that needs only these types +//! does not pull in a module of items it never names. //! //! # Examples //! //! ```no_run //! let job = workflow_model::Job::default(); //! assert!(!job.is_github_hosted()); +//! assert!(!job.runs_on.names_a_runner()); //! ``` use std::{collections::BTreeMap, fmt}; -/// Directory holding the repository's workflow definitions. -pub const WORKFLOW_DIR: &str = ".github/workflows"; - -/// Commit that every `actions/cache` reference must pin (v6.1.0). -pub const CACHE_ACTION_SHA: &str = "55cc8345863c7cc4c66a329aec7e433d2d1c52a9"; - -/// Commit that every `leynos/shared-actions` reference must pin. -pub const SHARED_ACTIONS_SHA: &str = "7d46a399558914f5a05074e55a560fec0269fd0d"; - -/// Runner label used by this repository's Ubicloud build and test jobs. -pub const UBICLOUD_LABEL: &str = "ubicloud-standard-8"; - -/// Jobs that build or test the crate and therefore keep an Ubicloud label. -pub const BUILD_JOB_IDS: [&str; 2] = ["build-test", "coverage-upload"]; - -/// Failure encountered while reading or parsing the workflow estate. -#[derive(Debug)] -pub enum WorkflowError { - /// A workflow file or the workflow directory could not be read. - Read(String, std::io::Error), - /// A workflow file was not valid YAML. - Parse(String, serde_norway::Error), - /// A workflow file was structurally unusable. - Shape(String, String), +/// How a job selects the runner it executes on. +/// +/// GitHub Actions accepts three shapes for `runs-on`: a single label, a +/// sequence of labels a runner must carry all of, and a mapping naming a +/// runner group with optional labels. Modelling only the scalar would make the +/// other two shapes parse errors, so a perfectly valid workflow would fail the +/// contracts instead of the workflow that deserves to. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub enum RunnerSelection { + /// The job names no runner because it calls a reusable workflow. + #[default] + Delegated, + /// Labels a runner must carry, from a scalar or a sequence. + Labels(Vec), + /// A runner group, with the labels required within that group. + Group { + /// Name of the runner group. + group: String, + /// Labels required within the group, possibly empty. + labels: Vec, + }, } -impl fmt::Display for WorkflowError { - /// Renders the failure with the workflow name that produced it. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl RunnerSelection { + /// Returns the labels the selection requires, empty when it names none. + #[must_use] + pub fn labels(&self) -> &[String] { match self { - Self::Read(name, err) => write!(f, "cannot read {name}: {err}"), - Self::Parse(name, err) => write!(f, "cannot parse {name}: {err}"), - Self::Shape(name, msg) => write!(f, "unexpected shape in {name}: {msg}"), + Self::Delegated => &[], + Self::Labels(labels) | Self::Group { labels, .. } => labels, } } -} - -impl std::error::Error for WorkflowError {} -/// Where in the estate a value was read, carried instead of a bare string so -/// the parsing helpers take one string argument rather than several. -#[derive(Debug, Clone)] -pub struct Location(String); - -impl Location { - /// Locates a whole workflow file. - #[must_use] - pub fn file(name: &str) -> Self { - Self(name.to_owned()) - } - - /// Locates one job within this file. + /// Reports whether the job names a runner of its own. #[must_use] - pub fn job(&self, id: &str) -> Self { - Self(format!("{}: job `{id}`", self.0)) - } - - /// Builds a shape error reported at this location. - #[must_use] - pub fn shape(&self, message: &str) -> WorkflowError { - WorkflowError::Shape(self.0.clone(), message.to_owned()) + pub const fn names_a_runner(&self) -> bool { + !matches!(self, Self::Delegated) } } -/// A workflow document paired with the file name it came from. -#[derive(Debug, Clone, Copy)] -pub struct WorkflowSource<'a> { - /// File name within [`WORKFLOW_DIR`]. - pub file: &'a str, - /// The document's YAML text. - pub text: &'a str, +impl fmt::Display for RunnerSelection { + /// Renders the selection the way a failure message should quote it. + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Delegated => write!(f, "(reusable workflow)"), + Self::Labels(labels) => write!(f, "{}", labels.join(", ")), + Self::Group { group, labels } if labels.is_empty() => write!(f, "group {group}"), + Self::Group { group, labels } => { + write!(f, "group {group} ({})", labels.join(", ")) + } + } + } } /// One step of a workflow job, reduced to the fields the contracts inspect. @@ -138,21 +119,38 @@ impl Step { pub struct Job { /// Key under the workflow's `jobs` mapping. pub id: String, - /// Runner label, or an empty string when the job calls a reusable workflow. - pub runs_on: String, + /// How the job selects its runner. + pub runs_on: RunnerSelection, /// Reusable workflow reference, or an empty string for a normal job. pub uses: String, /// Declared `timeout-minutes`, when present. pub timeout_minutes: Option, + /// Job-level environment, rendered as GitHub would export it. + pub env: BTreeMap, /// Steps in declaration order. pub steps: Vec, } impl Job { + /// Returns a job-level environment value, or an empty string when unset. + #[must_use] + pub fn env(&self, key: &str) -> &str { + self.env.get(key).map_or("", String::as_str) + } + /// Reports whether the job runs on a GitHub-hosted Ubuntu runner. + /// + /// A runner group is never GitHub-hosted, and a label set is only when + /// every label in it is one of GitHub's Ubuntu images: a job that also + /// requires a self-hosted label runs somewhere else. #[must_use] pub fn is_github_hosted(&self) -> bool { - self.runs_on.starts_with("ubuntu-") + match &self.runs_on { + RunnerSelection::Labels(labels) => { + !labels.is_empty() && labels.iter().all(|label| label.starts_with("ubuntu-")) + } + RunnerSelection::Delegated | RunnerSelection::Group { .. } => false, + } } /// Returns the first step whose `run` or `uses` text contains `needle`. @@ -163,23 +161,25 @@ impl Job { .position(|step| step.run.contains(needle) || step.uses.contains(needle)) } - /// Returns the first step whose `uses` names `action`, ignoring its pin. + /// Returns the first step matching `needle`, with its index. #[must_use] - pub fn step_using(&self, action: &str) -> Option<&Step> { - self.steps.iter().find(|step| { - step.uses - .split('@') - .next() - .is_some_and(|path| path.ends_with(action)) - }) + pub fn first_step_with(&self, needle: &str) -> Option<(usize, &Step)> { + self.steps + .iter() + .enumerate() + .find(|(_, step)| step.run.contains(needle) || step.uses.contains(needle)) } -} -/// One workflow file. -#[derive(Debug, Clone)] -pub struct Workflow { - /// File name within [`WORKFLOW_DIR`]. - pub file: String, - /// Jobs in declaration order. - pub jobs: Vec, + /// Returns the first step whose `uses` is `coordinate`, ignoring its pin. + /// + /// `coordinate` is the whole reference before the `@`, publisher included. + /// A suffix match would accept `untrusted/setup-rust@` wherever the + /// contracts ask for the shared `setup-rust`, so an action from the wrong + /// publisher could satisfy a policy check written to exclude it. + #[must_use] + pub fn step_using(&self, coordinate: &str) -> Option<&Step> { + self.steps + .iter() + .find(|step| step.uses.split('@').next() == Some(coordinate)) + } } diff --git a/tests/workflow_contracts.rs b/tests/workflow_contracts.rs index 5f8e4d5..3f54912 100644 --- a/tests/workflow_contracts.rs +++ b/tests/workflow_contracts.rs @@ -1,33 +1,39 @@ //! Structural contracts over the repository's GitHub Actions workflows. //! -//! These tests encode the Ubicloud adoption rules that a reviewer would -//! otherwise have to re-check by hand on every workflow edit: no tool is built -//! from source, every cached path has exactly one owner, cache and shared -//! action references are pinned, API-bound jobs stay GitHub-hosted, and an -//! installer always precedes the first use of what it installs. They also pin -//! the inputs that make those rules true, so a workflow cannot keep the shape -//! of the policy while dropping its substance. -//! -//! They read the workflow files directly, so they fail on the change that -//! introduces a violation rather than on the CI run that suffers from it. +//! These encode the Ubicloud adoption rules a reviewer would otherwise re-check +//! by hand on every workflow edit: no tool is built from source, every cached +//! path has one owner, references are pinned, API-bound jobs stay +//! GitHub-hosted, and an installer precedes the first use of what it installs. +//! They also pin the inputs that make those rules true, so a workflow cannot +//! keep the shape of the policy while dropping its substance. They read the +//! files directly, so they fail on the change that introduces a violation +//! rather than on the CI run that suffers from it. +#[path = "support/workflow_assertions.rs"] +mod workflow_assertions; #[path = "support/workflow_cache_owners.rs"] mod workflow_cache_owners; +#[path = "support/workflow_estate.rs"] +mod workflow_estate; #[path = "support/workflow_loader.rs"] mod workflow_loader; #[path = "support/workflow_model.rs"] mod workflow_model; use camino::Utf8Path; -use rstest::{fixture, rstest}; +use rstest::rstest; -use workflow_loader::{ - all_steps, load_workflows, load_workflows_in, parse_workflow, read_repository_file, -}; -use workflow_model::{ - Job, Step, Workflow, WorkflowSource, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_SHA, - UBICLOUD_LABEL, +use workflow_assertions::{assert_input, job_named, jobs, step_using, workflows}; +use workflow_estate::{ + Workflow, WorkflowSource, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_OWNER, + SHARED_ACTIONS_SHA, UBICLOUD_LABEL, }; +use workflow_loader::{all_steps, load_workflows_in, parse_workflow, read_repository_file}; + +/// Full coordinate of a shared composite action this repository calls. +fn shared_action(name: &str) -> String { + format!("{SHARED_ACTIONS_OWNER}/.github/actions/{name}") +} /// Fragments that mark a step as building a tool from source. /// @@ -39,6 +45,19 @@ const SOURCE_BUILD_FRAGMENTS: [&str; 3] = ["cargo install", "cargo-binstall ", " /// Commands that would run the test suite a second time in a build job. const REPEAT_TEST_COMMANDS: [&str; 4] = ["cargo test", "cargo nextest", "make test", "make all"]; +/// Commit that every `actions/github-script` reference must pin (v8). +const GITHUB_SCRIPT_SHA: &str = "ed597411d8f924073f98dfc5c65a23a2325f34cd"; + +/// Pinned prebuilt sccache the build jobs install. +const SCCACHE_TOOL: &str = "sccache@0.16.0"; + +/// Variables sccache's GitHub Actions backend needs re-exported on Ubicloud. +const PROXY_VARIABLES: [&str; 3] = [ + "ACTIONS_CACHE_URL", + "ACTIONS_RUNTIME_TOKEN", + "ACTIONS_CACHE_SERVICE_V2", +]; + /// Expression fragments the uv tool-layer cache key must carry. const UV_CACHE_KEY_FRAGMENTS: [&str; 4] = [ "runner.os", @@ -47,50 +66,6 @@ const UV_CACHE_KEY_FRAGMENTS: [&str; 4] = [ "hashFiles(", ]; -/// Every workflow in `.github/workflows`, parsed once per test. -#[fixture] -fn workflows() -> Vec { - load_workflows().unwrap_or_else(|err| panic!("workflow estate must parse: {err}")) -} - -/// Returns every job in the estate, tagged with its workflow file. -fn jobs(workflows: &[Workflow]) -> Vec<(String, Job)> { - workflows - .iter() - .flat_map(|workflow| { - workflow - .jobs - .iter() - .map(move |job| (workflow.file.clone(), job.clone())) - }) - .collect() -} - -/// Returns the job with the given id, or panics naming the missing job. -fn job_named<'a>(workflows: &'a [Workflow], id: &str) -> &'a Job { - workflows - .iter() - .flat_map(|workflow| workflow.jobs.iter()) - .find(|job| job.id == id) - .unwrap_or_else(|| panic!("workflow estate must define the `{id}` job")) -} - -/// Returns a job's step that uses `action`, or panics naming both. -fn step_using<'a>(job: &'a Job, action: &str) -> &'a Step { - job.step_using(action) - .unwrap_or_else(|| panic!("`{}` must use the `{action}` action", job.id)) -} - -/// Asserts that a step supplies the expected value for one input. -fn assert_input(job_id: &str, step: &Step, key: &str, expected: &str) { - assert_eq!( - step.input(key), - expected, - "`{job_id}` step `{}` must set `{key}: {expected}`", - step.label() - ); -} - #[rstest] fn every_cache_reference_is_pinned_to_v6_1_0(workflows: Vec) { let unpinned: Vec = all_steps(&workflows) @@ -190,7 +165,7 @@ fn each_cached_path_has_exactly_one_owner(workflows: Vec) { fn non_build_jobs_stay_on_github_hosted_runners(workflows: Vec) { let misplaced: Vec = jobs(&workflows) .into_iter() - .filter(|(_, job)| !job.runs_on.is_empty()) + .filter(|(_, job)| job.runs_on.names_a_runner()) .filter(|(_, job)| !BUILD_JOB_IDS.contains(&job.id.as_str())) .filter(|(_, job)| !job.is_github_hosted()) .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) @@ -216,7 +191,8 @@ fn build_jobs_keep_their_label_and_a_bounded_timeout( ) { let job = job_named(&workflows, id); assert_eq!( - job.runs_on, UBICLOUD_LABEL, + job.runs_on.labels(), + [UBICLOUD_LABEL], "`{id}` must keep its measured runner label" ); let timeout = job @@ -228,14 +204,34 @@ fn build_jobs_keep_their_label_and_a_bounded_timeout( ); } +/// A warm run has to be triggerable without pushing a commit, so the runner +/// and cache changes can be measured on an unchanged tree. +#[rstest] +fn the_pull_request_workflow_accepts_a_warm_run_dispatch(workflows: Vec) { + let Some(ci) = workflows.iter().find(|workflow| workflow.file == "ci.yml") else { + panic!("the estate must define ci.yml") + }; + assert!( + ci.has_trigger("workflow_dispatch"), + "ci.yml must accept `workflow_dispatch` so a warm run can be measured \ + on demand; it declares {:?}", + ci.triggers + ); +} + #[rstest] fn every_runner_label_is_registered_with_actionlint(workflows: Vec) { let text = read_repository_file(".github/actionlint.yaml") .unwrap_or_else(|err| panic!("actionlint configuration must be readable: {err}")); let unregistered: Vec = jobs(&workflows) .into_iter() - .filter(|(_, job)| !job.runs_on.is_empty() && !job.is_github_hosted()) - .filter(|(_, job)| !text.contains(job.runs_on.as_str())) + .filter(|(_, job)| job.runs_on.names_a_runner() && !job.is_github_hosted()) + .filter(|(_, job)| { + !job.runs_on + .labels() + .iter() + .all(|label| text.contains(label.as_str())) + }) .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) .collect(); assert!( @@ -286,20 +282,156 @@ fn coverage_is_the_only_test_execution(workflows: Vec) { } #[rstest] -fn setup_rust_owns_the_cargo_registry_and_installs_no_compiler_cache(workflows: Vec) { +fn setup_rust_owns_the_registry_but_not_the_compiler_cache(workflows: Vec) { for id in BUILD_JOB_IDS { let job = job_named(&workflows, id); - let step = step_using(job, "setup-rust"); + let step = step_using(job, &shared_action("setup-rust")); assert_input(id, step, "cache-provider", "github"); + // The action would start the sccache server inside an action step, + // where the Ubicloud runner re-injects its own cache variables and the + // server binds GitHub's v2 service instead of the local proxy. The job + // installs and starts sccache itself instead. assert_input(id, step, "use-sccache", "false"); } } +/// The two variables that make the wrapper more than overhead. +/// +/// `RUSTC_WRAPPER` engages sccache; `SCCACHE_GHA_ENABLED` selects the Actions +/// backend. Without the second, sccache writes to a local directory nothing +/// persists between runs, and every compilation misses. +#[rstest] +#[case::wrapper("RUSTC_WRAPPER", "sccache")] +#[case::backend("SCCACHE_GHA_ENABLED", "true")] +#[case::no_incremental("CARGO_INCREMENTAL", "0")] +fn the_compiler_cache_is_engaged_at_job_level( + workflows: Vec, + #[case] variable: &str, + #[case] expected: &str, +) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + assert_eq!( + job.env(variable), + expected, + "`{id}` must export `{variable}: {expected}` at job level" + ); + } +} + +#[rstest] +fn sccache_is_installed_from_a_pinned_prebuilt_release(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let step = step_using(job, "taiki-e/install-action"); + assert_input(id, step, "tool", SCCACHE_TOOL); + assert_input(id, step, "fallback", "none"); + } +} + +/// The sccache server binds its backend once, when it starts, so the order of +/// these steps is the contract. Started before the export it binds GitHub's v2 +/// service instead of Ubicloud's proxy; started after the toolchain is in +/// place it can miss the first compilation; reported before the build it +/// measures nothing. +#[rstest] +fn the_compiler_cache_is_wired_in_the_only_order_that_works(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let stage = |needle: &str, what: &str| { + job.first_step_containing(needle) + .unwrap_or_else(|| panic!("`{id}` must {what}")) + }; + let export = stage("actions/github-script", "export the Ubicloud cache proxy"); + let install = stage("taiki-e/install-action", "install a pinned sccache"); + let start = stage("sccache --zero-stats", "start the compiler cache"); + // `setup-rust` stands for the first step that could compile: it puts + // the toolchain in place, and nothing before it runs cargo. + let toolchain = stage("setup-rust", "set up Rust before anything compiles"); + let coverage = stage("generate-coverage", "build the workspace under coverage"); + let report = stage("sccache --show-stats", "report compiler-cache statistics"); + let order = [ + ("export the cache proxy", export), + ("install sccache", install), + ("start sccache", start), + ("set up the toolchain", toolchain), + ("build", coverage), + ("report the statistics", report), + ]; + for ((earlier, before), (later, after)) in order.iter().zip(order.iter().skip(1)) { + assert!( + before < after, + "`{id}` must {earlier} (step {before}) before it can {later} (step {after})" + ); + } + } +} + +#[rstest] +fn the_cache_proxy_export_is_pinned_and_names_every_variable(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let (export_at, export) = job + .first_step_with("actions/github-script") + .unwrap_or_else(|| panic!("`{id}` must export the Ubicloud cache proxy")); + assert!( + export.uses.ends_with(GITHUB_SCRIPT_SHA), + "`{id}` must pin actions/github-script to {GITHUB_SCRIPT_SHA}" + ); + let checkout_at = job + .first_step_containing("actions/checkout") + .unwrap_or_else(|| panic!("`{id}` must check out the repository")); + assert!( + checkout_at < export_at, + "`{id}` must export the proxy after checkout" + ); + let script = export.input("script"); + for variable in PROXY_VARIABLES { + assert!( + script.contains(variable), + "`{id}` must export `{variable}` for sccache's backend" + ); + } + assert!( + !script.contains("ACTIONS_RESULTS_URL"), + "`{id}` must not export ACTIONS_RESULTS_URL; it does not route \ + through Ubicloud's cache proxy" + ); + } +} + +#[rstest] +fn compiler_cache_effectiveness_is_measured_around_the_build(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let zero_at = job + .first_step_containing("sccache --zero-stats") + .unwrap_or_else(|| panic!("`{id}` must reset the compiler-cache counters")); + let (show_at, report) = job + .first_step_with("sccache --show-stats") + .unwrap_or_else(|| panic!("`{id}` must report compiler-cache statistics")); + assert!( + zero_at < show_at, + "`{id}` must reset the counters before it reports them" + ); + assert!( + report.run.contains("GITHUB_STEP_SUMMARY"), + "`{id}` must put the compiler-cache statistics in the job summary" + ); + // The summary is not readable through the REST API, so a run whose + // statistics went only there cannot be audited afterwards. + assert!( + report.run.contains("printf '%s\\n' \"$stats\""), + "`{id}` must also print the compiler-cache statistics to the log" + ); + } +} + #[rstest] fn coverage_runs_the_whole_suite_once_and_owns_no_cargo_cache(workflows: Vec) { for id in BUILD_JOB_IDS { let job = job_named(&workflows, id); - let step = step_using(job, "generate-coverage"); + let step = step_using(job, &shared_action("generate-coverage")); for flag in ["all-features", "all-targets", "doctests"] { assert_input(id, step, flag, "true"); } @@ -310,7 +442,7 @@ fn coverage_runs_the_whole_suite_once_and_owns_no_cargo_cache(workflows: Vec) { let job = job_named(&workflows, "build-test"); - let step = step_using(job, "install-whitaker"); + let step = step_using(job, &shared_action("install-whitaker")); assert_input("build-test", step, "installer-version", "0.2.7"); assert_input("build-test", step, "cache-provider", "github"); } @@ -318,11 +450,13 @@ fn whitaker_is_installed_from_a_pinned_prebuilt_release(workflows: Vec #[rstest] fn the_uv_cache_names_its_layers_and_keys_them_by_runner(workflows: Vec) { let job = job_named(&workflows, "build-test"); - let step = job + let cache = job .steps .iter() - .find(|step| step.cache_paths().iter().any(|path| path == ".uv-cache")) - .unwrap_or_else(|| panic!("`build-test` must cache the uv download layer")); + .find(|step| step.cache_paths().iter().any(|path| path == ".uv-cache")); + let Some(step) = cache else { + panic!("`build-test` must cache the uv download layer") + }; assert_eq!( step.cache_paths(), vec![".uv-cache".to_owned(), ".uv-tools".to_owned()], @@ -339,7 +473,9 @@ fn the_uv_cache_names_its_layers_and_keys_them_by_runner(workflows: Vec Step { fn job_of(steps: Vec) -> Job { Job { id: "j".to_owned(), - runs_on: "ubuntu-latest".to_owned(), + runs_on: RunnerSelection::Labels(vec!["ubuntu-latest".to_owned()]), steps, ..Job::default() } @@ -156,6 +158,37 @@ proptest! { prop_assert_eq!(reported(&job), reported(&job_of(rotated))); } + /// Two restores sharing a key are two owners, not one half of a pair. + /// + /// The split-cache exception exists for one restore and one save. Applying + /// it to any step whose key matched would let a genuine duplicate hide + /// behind it. + #[test] + fn two_restores_sharing_a_key_are_two_owners( + path in prop::sample::select(PATHS.to_vec()), + filler in prop::collection::vec(any_step(), 0..4), + ) { + let mut steps = vec![split_step("restore", path, "k1")]; + steps.extend(without_claims_on(filler, path)); + steps.push(split_step("restore", path, "k1")); + prop_assert!(reported(&job_of(steps)).contains(path)); + } + + /// A third step on a paired key breaks the pair rather than joining it. + #[test] + fn an_extra_restore_beside_a_matching_pair_is_a_duplicate( + path in prop::sample::select(PATHS.to_vec()), + filler in prop::collection::vec(any_step(), 0..4), + ) { + let mut steps = vec![ + split_step("restore", path, "k1"), + split_step("save", path, "k1"), + ]; + steps.extend(without_claims_on(filler, path)); + steps.push(split_step("restore", path, "k1")); + prop_assert!(reported(&job_of(steps)).contains(path)); + } + /// A restore and a save sharing a key are one owner; differing keys are two. #[test] fn a_split_cache_is_one_owner_only_when_its_halves_agree( From e5f321b5df93b3651570c4c78c6fc1fa312517f4 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Fri, 4 Sep 2026 13:43:55 +0100 Subject: [PATCH 08/10] Measure the runner shape instead of inheriting it `ubicloud-standard-8` was chosen before this rollout and has never been measured here, so there is no evidence for keeping it or for shrinking it. Both build jobs now start a background sampler after checkout that records used memory and used and free disk every 15 seconds, and report peak memory, peak disk and least free disk at the end of the job, to the log as well as the job summary. Disk is sampled alongside memory because disk is what has actually exhausted runners in this estate, and it did so with no error text: a step simply stopped. A contract asserts that both jobs sample and report, and that the report names disk rather than memory alone. Repin every shared action to 3a2f2d5f, which restores the cache service the sccache steps overwrite. The jobs keep starting sccache from a `run:` step; adopting the action's own path is a later wave. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- .github/workflows/ci.yml | 59 ++++++++++++++++++++-- .github/workflows/coverage-main.yml | 57 +++++++++++++++++++-- .github/workflows/dependabot-automerge.yml | 2 +- docs/developers-guide.md | 19 +++++-- tests/support/workflow_cache_owners.rs | 2 +- tests/support/workflow_estate.rs | 2 +- tests/workflow_contracts.rs | 30 +++++++++++ 7 files changed, 157 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bbfd195..303d75f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,32 @@ jobs: # (`upload-codescene-coverage` with `mode: check`) can reach # the pull request's merge base. fetch-depth: 0 + # Ubicloud's `standard-8` label is inherited here, never measured. Sample + # memory and disk every 15 s so the next shape decision rests on evidence + # rather than on what a previous change happened to pick. Disk, not + # memory, is what has killed jobs elsewhere in this rollout, and it did so + # silently, so both are recorded. + - name: Start the resource sampler + shell: bash + env: + RESOURCE_SAMPLES: ${{ runner.temp }}/resource-samples.txt + run: | + set -euo pipefail + : > "$RESOURCE_SAMPLES" + sampler="$RUNNER_TEMP/sample-resources.sh" + cat > "$sampler" <<'SAMPLER' + #!/usr/bin/env bash + set -uo pipefail + while :; do + mem_used="$(free -m | awk '/^Mem:/ { print $3 }')" + disk="$(df -m --output=used,avail / | tail -1)" + printf '%s %s\n' "$mem_used" "$disk" >> "$1" + sleep 15 + done + SAMPLER + chmod +x "$sampler" + nohup "$sampler" "$RESOURCE_SAMPLES" >/dev/null 2>&1 & + printf 'RESOURCE_SAMPLES=%s\n' "$RESOURCE_SAMPLES" >> "$GITHUB_ENV" - name: Route the compiler cache into Ubicloud's store # sccache's GitHub Actions backend reads these from the environment, # but the runner exposes them to action code rather than to later @@ -92,7 +118,7 @@ jobs: sccache --version sccache --zero-stats - name: Setup Rust - uses: leynos/shared-actions/.github/actions/setup-rust@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/actions/setup-rust@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: # Sole owner of ~/.cargo/registry and ~/.cargo/git for this job. It # runs before the first cargo invocation so the lint step reads a @@ -124,7 +150,7 @@ jobs: # Downloads the pinned prebuilt installer and verifies it against a # digest pinned in the action. The action owns the cache for the # installer binary, its version marker, and ~/.local/share/whitaker. - uses: leynos/shared-actions/.github/actions/install-whitaker@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/actions/install-whitaker@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: installer-version: '0.2.7' cache-provider: github @@ -137,7 +163,7 @@ jobs: # evidence and doubled the billed compile. `all-features` names exactly # the set the explicit feature list used to name. - name: Generate coverage - uses: leynos/shared-actions/.github/actions/generate-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/actions/generate-coverage@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: all-features: 'true' all-targets: 'true' @@ -160,6 +186,31 @@ jobs: df -h . rm -rf -- target/llvm-cov-target df -h . + - name: Report peak resource use + if: always() + shell: bash + run: | + set -euo pipefail + samples="${RESOURCE_SAMPLES:-}" + if [[ -z "$samples" || ! -s "$samples" ]]; then + echo 'No resource samples were recorded.' + exit 0 + fi + peak_memory="$(awk '{ print $1 }' "$samples" | sort -n | tail -1)" + peak_disk="$(awk '{ print $2 }' "$samples" | sort -n | tail -1)" + least_free="$(awk '{ print $3 }' "$samples" | sort -n | head -1)" + count="$(wc -l < "$samples")" + printf 'peak used memory: %s MiB\n' "$peak_memory" + printf 'peak used disk: %s MiB\n' "$peak_disk" + printf 'least free disk: %s MiB\n' "$least_free" + printf 'samples: %s at 15 second intervals\n' "$count" + { + printf '### Resources (%s)\n\n' "${GITHUB_JOB}" + printf -- '- peak used memory: %s MiB\n' "$peak_memory" + printf -- '- peak used disk: %s MiB\n' "$peak_disk" + printf -- '- least free disk: %s MiB\n' "$least_free" + printf -- '- samples: %s at 15 second intervals\n' "$count" + } >> "${GITHUB_STEP_SUMMARY}" - name: Record compiler-cache effectiveness if: always() shell: bash @@ -185,7 +236,7 @@ jobs: env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} if: env.CS_ACCESS_TOKEN != '' && github.event_name == 'pull_request' - uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: format: lcov mode: check diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index 580b4c5..ce6d15c 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -33,6 +33,32 @@ jobs: CARGO_INCREMENTAL: '0' steps: - uses: actions/checkout@v7 + # Ubicloud's `standard-8` label is inherited here, never measured. Sample + # memory and disk every 15 s so the next shape decision rests on evidence + # rather than on what a previous change happened to pick. Disk, not + # memory, is what has killed jobs elsewhere in this rollout, and it did so + # silently, so both are recorded. + - name: Start the resource sampler + shell: bash + env: + RESOURCE_SAMPLES: ${{ runner.temp }}/resource-samples.txt + run: | + set -euo pipefail + : > "$RESOURCE_SAMPLES" + sampler="$RUNNER_TEMP/sample-resources.sh" + cat > "$sampler" <<'SAMPLER' + #!/usr/bin/env bash + set -uo pipefail + while :; do + mem_used="$(free -m | awk '/^Mem:/ { print $3 }')" + disk="$(df -m --output=used,avail / | tail -1)" + printf '%s %s\n' "$mem_used" "$disk" >> "$1" + sleep 15 + done + SAMPLER + chmod +x "$sampler" + nohup "$sampler" "$RESOURCE_SAMPLES" >/dev/null 2>&1 & + printf 'RESOURCE_SAMPLES=%s\n' "$RESOURCE_SAMPLES" >> "$GITHUB_ENV" - name: Route the compiler cache into Ubicloud's store # See ci.yml for why each variable is exported, and why this must # precede the step that starts the sccache server. @@ -69,7 +95,7 @@ jobs: sccache --version sccache --zero-stats - name: Setup Rust - uses: leynos/shared-actions/.github/actions/setup-rust@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/actions/setup-rust@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: # Sole owner of ~/.cargo/registry and ~/.cargo/git for this job, and # the trunk writer whose entry every pull-request run restores. @@ -77,7 +103,7 @@ jobs: # The job installs and starts sccache itself, above. use-sccache: 'false' - name: Generate coverage - uses: leynos/shared-actions/.github/actions/generate-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/actions/generate-coverage@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: all-features: 'true' all-targets: 'true' @@ -97,6 +123,31 @@ jobs: df -h . rm -rf -- target/llvm-cov-target df -h . + - name: Report peak resource use + if: always() + shell: bash + run: | + set -euo pipefail + samples="${RESOURCE_SAMPLES:-}" + if [[ -z "$samples" || ! -s "$samples" ]]; then + echo 'No resource samples were recorded.' + exit 0 + fi + peak_memory="$(awk '{ print $1 }' "$samples" | sort -n | tail -1)" + peak_disk="$(awk '{ print $2 }' "$samples" | sort -n | tail -1)" + least_free="$(awk '{ print $3 }' "$samples" | sort -n | head -1)" + count="$(wc -l < "$samples")" + printf 'peak used memory: %s MiB\n' "$peak_memory" + printf 'peak used disk: %s MiB\n' "$peak_disk" + printf 'least free disk: %s MiB\n' "$least_free" + printf 'samples: %s at 15 second intervals\n' "$count" + { + printf '### Resources (%s)\n\n' "${GITHUB_JOB}" + printf -- '- peak used memory: %s MiB\n' "$peak_memory" + printf -- '- peak used disk: %s MiB\n' "$peak_disk" + printf -- '- least free disk: %s MiB\n' "$least_free" + printf -- '- samples: %s at 15 second intervals\n' "$count" + } >> "${GITHUB_STEP_SUMMARY}" - name: Record compiler-cache effectiveness if: always() shell: bash @@ -116,7 +167,7 @@ jobs: env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} if: env.CS_ACCESS_TOKEN != '' - uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: format: lcov access-token: ${{ env.CS_ACCESS_TOKEN }} diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml index 90d1aa1..95c6fcc 100644 --- a/.github/workflows/dependabot-automerge.yml +++ b/.github/workflows/dependabot-automerge.yml @@ -28,6 +28,6 @@ jobs: # The token is not used for any external cloud auth. id-token: write if: ${{ github.event_name == 'workflow_dispatch' || github.actor == 'dependabot[bot]' }} - uses: leynos/shared-actions/.github/workflows/dependabot-automerge.yml@c6125f19593668cbfefd65a59c08cb7aefe90d93 + uses: leynos/shared-actions/.github/workflows/dependabot-automerge.yml@3a2f2d5f17932657ddf50490a09ea5e7400ae35c with: pull-request-number: ${{ inputs.pull-request-number || github.event.pull_request.number }} diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 93eb773..339f37f 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -361,7 +361,7 @@ No tool is compiled from source. `whitaker-installer` is installed by pinned prebuilt release archive and verifies it against a digest pinned inside the action, then runs the installer to place the Whitaker Dylint suite. Every `leynos/shared-actions` reference pins commit -`c6125f19593668cbfefd65a59c08cb7aefe90d93`. +`3a2f2d5f17932657ddf50490a09ea5e7400ae35c`. sccache is installed the same way, by `taiki-e/install-action` with `tool: sccache@0.16.0` and `fallback: none`. The fallback matters: without it @@ -444,6 +444,16 @@ generated, printing `df -h` either side. The instrumented tree has no later consumer, and on smaller runner shapes a full disk has killed a job silently, with no error text. +### Resource sampling + +Both build jobs start a background sampler after checkout that records used +memory and used and free disk every 15 seconds, and report the peaks at the +end of the job. The `ubicloud-standard-8` label was inherited rather than +measured, so the samples are what a future decision to keep or shrink the shape +will rest on. Disk is sampled alongside memory because disk, not memory, is +what has exhausted runners in this estate, and it did so with no error text at +all. + `ci.yml` accepts `workflow_dispatch` so a warm run can be measured on demand. A dispatch restores what a pull request restores and writes nothing: `coverage-main.yml` is the only job that saves on this repository. @@ -476,9 +486,10 @@ single test execution per build job. It also pins the inputs that make those rules true, so a workflow cannot keep the shape of the policy while dropping its substance: `cache-provider`, `use-sccache`, the Whitaker installer version, the coverage flags, the uv cache paths and key, a bounded -`timeout-minutes` for each build job, and the compiler-cache wiring: the two -job-level variables, and the export, install, start, build, report order that -the sccache server's one-shot backend binding depends on. +`timeout-minutes` for each build job, the resource sampler and its report, and +the compiler-cache wiring: the two job-level variables, and the export, +install, start, build, report order that the sccache server's one-shot backend +binding depends on. `tests/support/workflow_model.rs` holds the job, step, and runner-selection types the properties and the contracts share; diff --git a/tests/support/workflow_cache_owners.rs b/tests/support/workflow_cache_owners.rs index 0218d1e..834dc9f 100644 --- a/tests/support/workflow_cache_owners.rs +++ b/tests/support/workflow_cache_owners.rs @@ -35,7 +35,7 @@ pub struct CacheOwner { /// Paths a shared composite action caches when `cache-provider` is `github`. /// /// These mirror the action definitions at -/// `leynos/shared-actions@c6125f19593668cbfefd65a59c08cb7aefe90d93`. A caller +/// `leynos/shared-actions@3a2f2d5f17932657ddf50490a09ea5e7400ae35c`. A caller /// that sets `cache-provider: external` takes the path away from the action, /// which is how a second owner of the Cargo registry is avoided. const SHARED_ACTION_CACHES: [(&str, &[&str]); 3] = [ diff --git a/tests/support/workflow_estate.rs b/tests/support/workflow_estate.rs index ed9dc7e..44daf59 100644 --- a/tests/support/workflow_estate.rs +++ b/tests/support/workflow_estate.rs @@ -23,7 +23,7 @@ pub const WORKFLOW_DIR: &str = ".github/workflows"; pub const CACHE_ACTION_SHA: &str = "55cc8345863c7cc4c66a329aec7e433d2d1c52a9"; /// Commit that every `leynos/shared-actions` reference must pin. -pub const SHARED_ACTIONS_SHA: &str = "c6125f19593668cbfefd65a59c08cb7aefe90d93"; +pub const SHARED_ACTIONS_SHA: &str = "3a2f2d5f17932657ddf50490a09ea5e7400ae35c"; /// Runner label used by this repository's Ubicloud build and test jobs. pub const UBICLOUD_LABEL: &str = "ubicloud-standard-8"; diff --git a/tests/workflow_contracts.rs b/tests/workflow_contracts.rs index 3f54912..1b32251 100644 --- a/tests/workflow_contracts.rs +++ b/tests/workflow_contracts.rs @@ -427,6 +427,36 @@ fn compiler_cache_effectiveness_is_measured_around_the_build(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let start = job + .first_step_containing("sample-resources.sh") + .unwrap_or_else(|| panic!("`{id}` must start a resource sampler")); + let (report_at, report) = job + .first_step_with("least free disk") + .unwrap_or_else(|| panic!("`{id}` must report its peak resource use")); + assert!( + start < report_at, + "`{id}` must start the sampler before it reports the peaks" + ); + for measure in ["free -m", "df -m"] { + assert!( + job.steps.iter().any(|step| step.run.contains(measure)), + "`{id}` must sample `{measure}`; disk and memory are both needed" + ); + } + assert!( + report.run.contains("peak used disk"), + "`{id}` must report peak disk, not memory alone" + ); + } +} + #[rstest] fn coverage_runs_the_whole_suite_once_and_owns_no_cargo_cache(workflows: Vec) { for id in BUILD_JOB_IDS { From 29e9b48aaac6a2c5887b86aba836d5b13606c2c3 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Fri, 4 Sep 2026 15:02:55 +0100 Subject: [PATCH 09/10] Name the right cause for the sccache workaround The comments and the guide blamed the Ubicloud runner for re-injecting its cache variables into every action step. That is not what happens. Measured on ubicloud-standard-2, `run:` steps do see what the credentials export wrote, so the export was never being hidden from them. The actual cause is narrower and sits in one action. `setup-rust` with `use-sccache: 'true'` runs the mozilla sccache-action, and that action's last act writes `ACTIONS_CACHE_SERVICE_V2=on`, GitHub's results URL and GitHub's token back to `GITHUB_ENV`. Every later step then sees GitHub's v2 cache service instead of Ubicloud's proxy. The wiring is unchanged, because the fix is the same either way: start the server from a `run:` step before anything can clobber the endpoint it reads. Only the reason given for it changes, and the reason is what a later reader will act on. Record the shape evidence the samplers produced. `ubicloud-standard-8` is inherited here and has never been measured, and the first samples give 8,812 MiB peak memory against 101,691 MiB least free disk, so memory is the binding constraint: too large for standard-2's 8 GB, comfortable inside standard-4's 16 GB. That is not enough to shrink it. Halving the vCPU count trades wall time against the rate, and a Bevy workspace is where that bites, so the guide records the rule instead: measure two warm runs on main after this lands, and open the follow-up only if the second comes in under 25 minutes. Claude-Session: https://claude.ai/code/session_01QrjNTnTwM7FmWXe5KFPMPY --- .github/workflows/ci.yml | 21 ++++++----- .github/workflows/coverage-main.yml | 6 +-- docs/developers-guide.md | 57 ++++++++++++++++++++++------- tests/workflow_contracts.rs | 12 +++--- 4 files changed, 65 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 303d75f..f504426 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,12 +107,15 @@ jobs: tool: sccache@0.16.0 fallback: none - name: Reset compiler-cache counters - # This starts the sccache server, and it must be a `run:` step. The - # Ubicloud runner re-injects `ACTIONS_CACHE_SERVICE_V2=on` and - # `ACTIONS_RESULTS_URL` into every action step, overriding what the - # export above wrote to `GITHUB_ENV`, so a server started inside an - # action binds GitHub's v2 service and its writes never reach - # Ubicloud's store. A `run:` step sees only `GITHUB_ENV`. + # This starts the sccache server, and starting it here rather than + # inside `setup-rust` is the point. The shared action's sccache path + # runs the mozilla sccache-action, whose last act writes + # `ACTIONS_CACHE_SERVICE_V2=on`, GitHub's results URL, and GitHub's + # token back to `GITHUB_ENV`, clobbering the export above for every + # later step. Measured on `ubicloud-standard-2`: `run:` steps do see + # the export, so the export itself was never the problem. Starting the + # server here means it binds the proxy before anything can overwrite + # the endpoint it read. run: | set -euo pipefail sccache --version @@ -124,9 +127,9 @@ jobs: # runs before the first cargo invocation so the lint step reads a # warm registry. cache-provider: github - # The job installs and starts sccache itself, above. Letting this - # action do it would start the server inside an action step, where - # the runner's re-injected variables bind GitHub's v2 service. + # The job installs and starts sccache itself, above. This action's + # sccache path would rewrite `GITHUB_ENV` back to GitHub's v2 cache + # service on its way out, so every later step would lose the proxy. use-sccache: 'false' - name: Cache uv tool layers # `make spelling` drives uv with repository-local UV_CACHE_DIR and diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index ce6d15c..0dcf4e7 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -87,9 +87,9 @@ jobs: tool: sccache@0.16.0 fallback: none - name: Reset compiler-cache counters - # A `run:` step, for the reason given in ci.yml: a server started - # inside an action step binds GitHub's v2 service instead of the - # Ubicloud proxy. + # Started here rather than inside `setup-rust`, for the reason given + # in ci.yml: the shared action's sccache path rewrites `GITHUB_ENV` + # back to GitHub's v2 cache service on its way out. run: | set -euo pipefail sccache --version diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 339f37f..e09a6a1 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -422,14 +422,22 @@ action step is safe here because installing a binary does not start the sccache server. **Start.** A `run:` step runs `sccache --zero-stats`, which starts the server. -This must be a `run:` step and it must follow the export. The Ubicloud runner -re-injects `ACTIONS_CACHE_SERVICE_V2=on` and `ACTIONS_RESULTS_URL` into every -action step, overriding what the export wrote to `GITHUB_ENV`. A server started -inside an action step, which is what `setup-rust` with `use-sccache: 'true'` -would do, therefore binds GitHub's v2 service; its writes then fail and nothing -reaches Ubicloud's store. The server binds its backend once, at start, so a -later change to the environment is invisible to it. That is why `setup-rust` is -called with `use-sccache: 'false'` in both jobs. +It must follow the export, and it must not be `setup-rust`'s job. The reason is +narrower than it first looks, and the obvious guess is wrong: `run:` steps do +see what the export wrote, measured on `ubicloud-standard-2`, so the export is +not being hidden from them. What happens is that `setup-rust` with +`use-sccache: 'true'` runs the mozilla sccache-action, and that action's last +act writes `ACTIONS_CACHE_SERVICE_V2=on`, GitHub's results URL and GitHub's +token back to `GITHUB_ENV`. Every step after it therefore sees GitHub's v2 +cache service instead of Ubicloud's proxy, and a server started under those +values writes where nothing is reading. The server binds its backend once, at +start, so starting it before that clobbering happens is what makes it stick. +Hence `use-sccache: 'false'` in both jobs. + +The failure is silent and total, which is why it is worth this much text. +Before the fix, a run reported `Cache location: ghac`, an endpoint and a token +both present, and 8,170 write errors out of 8,170 writes. After it, the same +job reported 5 write errors in 5,442 and a 33.45 % hit rate. **Report.** `sccache --show-stats` runs after the build, printing the counters to the log as well as to the job summary. The log copy is the one that matters: @@ -447,13 +455,36 @@ with no error text. ### Resource sampling Both build jobs start a background sampler after checkout that records used -memory and used and free disk every 15 seconds, and report the peaks at the -end of the job. The `ubicloud-standard-8` label was inherited rather than -measured, so the samples are what a future decision to keep or shrink the shape -will rest on. Disk is sampled alongside memory because disk, not memory, is -what has exhausted runners in this estate, and it did so with no error text at +memory and used and free disk every 15 seconds, and report peak memory, peak +disk and least free disk at the end of the job, to the log as well as the job +summary. Disk is sampled alongside memory because disk, not memory, is what has +exhausted runners elsewhere in this estate, and it did so with no error text at all. +The `ubicloud-standard-8` label is inherited, not measured. It predates this +work and no evidence on this repository argued for it. The first samples, from +`build-test` on a cold cache, are: + +| Measure | Value | +| --- | --- | +| Peak used memory | 8,812 MiB | +| Peak used disk | 95,609 MiB | +| Least free disk | 101,691 MiB | +| Samples | 156 at 15 s intervals | + +Memory is the binding constraint. The 8.6 GiB peak rules out +`ubicloud-standard-2`, which has 8 GB, and fits inside `ubicloud-standard-4`, +which has 16 GB; free disk never fell below 99 GiB, so disk is not deciding +anything here. + +That is not sufficient to shrink the shape, because halving the vCPU count +trades wall time against the lower rate, and a Bevy workspace is where that +trade bites. Decide it on a warm cache, not a cold one. The rule is: after this +lands, the merge push is the cold writer on `main`, then run `ci.yml` twice +against `main` in sequence; if the second warm `build-test` comes in under 25 +minutes, open a follow-up moving both jobs to `ubicloud-standard-4` with the +samplers kept, and measure again. + `ci.yml` accepts `workflow_dispatch` so a warm run can be measured on demand. A dispatch restores what a pull request restores and writes nothing: `coverage-main.yml` is the only job that saves on this repository. diff --git a/tests/workflow_contracts.rs b/tests/workflow_contracts.rs index 1b32251..5bddf57 100644 --- a/tests/workflow_contracts.rs +++ b/tests/workflow_contracts.rs @@ -287,9 +287,9 @@ fn setup_rust_owns_the_registry_but_not_the_compiler_cache(workflows: Vec) /// The sccache server binds its backend once, when it starts, so the order of /// these steps is the contract. Started before the export it binds GitHub's v2 -/// service instead of Ubicloud's proxy; started after the toolchain is in -/// place it can miss the first compilation; reported before the build it -/// measures nothing. +/// service instead of Ubicloud's proxy; started after `setup-rust`, whose +/// sccache path rewrites the cache service back into `GITHUB_ENV`, it binds +/// whatever that left behind; reported before the build it measures nothing. #[rstest] fn the_compiler_cache_is_wired_in_the_only_order_that_works(workflows: Vec) { for id in BUILD_JOB_IDS { From 2ba6326326b054ba82171efa2ac9db5f65141347 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Fri, 4 Sep 2026 19:27:19 +0100 Subject: [PATCH 10/10] Stop the loose matches that let rules pass without checking Three of this round's findings share a shape: a match that was wider or narrower than the rule it served, so the rule reported success without having checked anything. Cache ownership matched `actions/cache` by prefix, so `actions/cache-audit` read as a cache step and contributed an invented claim on whatever `path` input it carried, which could report a duplicate that does not exist. Ownership now compares the three cache coordinates exactly. The pinning contract had the mirror-image bug, demanding that a prefix-sharing action pin the v6.1.0 cache SHA, and now uses the same predicate. Runner labels were checked by searching the raw text of `.github/actionlint.yaml`, which a substring satisfies: `standard-8` passed because `ubicloud-standard-8` contains it, and a commented-out registration passed too. Labels are now parsed from `self-hosted-runner.labels` and compared by equality. `parse_job` read `uses`, `runs-on` and `steps` independently, so a job that called a reusable workflow and also named a runner parsed cleanly, although GitHub Actions rejects that shape. It is now an error, on the presence of `steps` rather than its emptiness, because `steps: []` beside `uses` is exactly as invalid as steps with content. Guard the compiler-cache report in both jobs. `if: always()` is right there, because a failed build is when the counters are most worth having, but under `set -euo pipefail` a run that died before sccache was installed turned a missing binary into a second, misleading failure. The step now checks for sccache and exits cleanly when it is absent. Split the contract file, which had reached 569 lines against a 400-line cap. `workflow_contracts.rs` is now a harness over four modules named for the question each asks: what the estate will execute, what it costs and who owns each cache, whether sccache is actually working, and whether the loader reads workflows correctly. They stay in one test binary, which keeps every support item used and avoids the dead-code suppressions separate binaries would need. The loader had reached 397 lines in the process, which is not headroom, so its repository-file readers moved to `workflow_config.rs`; the loader reads `.github/workflows` and that module reads the other configuration a contract needs. Record the compiler cache's measured worth and the runner shape's first evidence in the guide. Three runs of `build-test` differing only in the shared-actions pin and in whether the store was populated: no hits and every write failing, then 33.45 % while populating, then 99.79 % reading it back at 16m31s against 25m44s uncached. The samplers put peak memory at 8,812 MiB on the cold writer and 6,815 MiB warm. The cold writer sets the floor, because it is the run that has to succeed, so standard-2 is out and standard-4 is the safe shrink; the guide records that rule and the criteria for accepting it. --- .github/workflows/ci.yml | 7 + .github/workflows/coverage-main.yml | 7 + docs/developers-guide.md | 157 ++++--- tests/contracts/compiler_cache.rs | 189 ++++++++ tests/contracts/parsing.rs | 90 ++++ tests/contracts/placement.rs | 197 +++++++++ tests/contracts/supply_chain.rs | 124 ++++++ tests/support/workflow_cache_owners.rs | 23 +- tests/support/workflow_config.rs | 72 ++++ tests/support/workflow_loader.rs | 39 +- tests/workflow_contracts.rs | 571 +------------------------ tests/workflow_model_properties.rs | 18 + 12 files changed, 873 insertions(+), 621 deletions(-) create mode 100644 tests/contracts/compiler_cache.rs create mode 100644 tests/contracts/parsing.rs create mode 100644 tests/contracts/placement.rs create mode 100644 tests/contracts/supply_chain.rs create mode 100644 tests/support/workflow_config.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f504426..7f9bef4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -219,6 +219,13 @@ jobs: shell: bash run: | set -euo pipefail + # `if: always()` runs this even when an earlier step failed before + # sccache was installed. Reporting nothing is correct there; failing + # here would bury the real failure under a second one. + if ! command -v sccache >/dev/null 2>&1; then + echo 'sccache is not installed; no compiler-cache statistics to report.' + exit 0 + fi # Print to the log as well as the job summary: the summary is not # readable through the REST API, so the log copy is what lets anyone # confirm `Cache location`, the hit rate, and any read or write diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index 0dcf4e7..a9ddd0c 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -153,6 +153,13 @@ jobs: shell: bash run: | set -euo pipefail + # `if: always()` runs this even when an earlier step failed before + # sccache was installed. Reporting nothing is correct there; failing + # here would bury the real failure under a second one. + if ! command -v sccache >/dev/null 2>&1; then + echo 'sccache is not installed; no compiler-cache statistics to report.' + exit 0 + fi # To the log as well as the summary: the summary is not readable # through the REST API. stats="$(sccache --show-stats)" diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e09a6a1..b2e65f0 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -435,9 +435,25 @@ start, so starting it before that clobbering happens is what makes it stick. Hence `use-sccache: 'false'` in both jobs. The failure is silent and total, which is why it is worth this much text. -Before the fix, a run reported `Cache location: ghac`, an endpoint and a token -both present, and 8,170 write errors out of 8,170 writes. After it, the same -job reported 5 write errors in 5,442 and a 33.45 % hit rate. +Three runs of `build-test` on the same shape, differing only in the +shared-actions pin and in whether the store had been populated, show both the +failure and what the cache is worth: + +| Measure | Before the fix | After, cold | After, warm | +| --- | --- | --- | --- | +| Cache location | ghac | ghac | ghac | +| Hit rate | 0.00 % | 33.45 % | 99.79 % | +| Rust hit rate | 0.00 % | 0.19 % | 99.60 % | +| Read errors | 0 | 0 | 0 | +| Write errors | 8170 | 5 | 0 | +| Wall | 25m44s | 39m14s | 16m31s | + +The first run had a correct backend, an endpoint and a token both present, and +every one of its 8,170 writes failed, so nothing reached the store and nothing +in the log said so except the write counter. The second populated the store, +which is why it is the slowest. The third reads what the second wrote. The +cache is worth about nine minutes a run on this workspace, 16m31s warm against +25m44s for the run that cached nothing at all. **Report.** `sccache --show-stats` runs after the build, printing the counters to the log as well as to the job summary. The log copy is the one that matters: @@ -463,27 +479,33 @@ all. The `ubicloud-standard-8` label is inherited, not measured. It predates this work and no evidence on this repository argued for it. The first samples, from -`build-test` on a cold cache, are: - -| Measure | Value | -| --- | --- | -| Peak used memory | 8,812 MiB | -| Peak used disk | 95,609 MiB | -| Least free disk | 101,691 MiB | -| Samples | 156 at 15 s intervals | - -Memory is the binding constraint. The 8.6 GiB peak rules out -`ubicloud-standard-2`, which has 8 GB, and fits inside `ubicloud-standard-4`, -which has 16 GB; free disk never fell below 99 GiB, so disk is not deciding -anything here. - -That is not sufficient to shrink the shape, because halving the vCPU count -trades wall time against the lower rate, and a Bevy workspace is where that -trade bites. Decide it on a warm cache, not a cold one. The rule is: after this -lands, the merge push is the cold writer on `main`, then run `ci.yml` twice -against `main` in sequence; if the second warm `build-test` comes in under 25 -minutes, open a follow-up moving both jobs to `ubicloud-standard-4` with the -samplers kept, and measure again. +`build-test`: + +| Measure | Cold writer | Warm | +| --- | --- | --- | +| Peak used memory | 8,812 MiB | 6,815 MiB | +| Peak used disk | 95,609 MiB | 94,521 MiB | +| Least free disk | 101,691 MiB | 102,779 MiB | +| Samples | 156 | 65 | + +Memory is the binding constraint, not disk: free disk never fell below 99 GiB +on either run. + +**The cold writer sets the memory floor, not the warm run.** The warm peak of +6,815 MiB would fit `ubicloud-standard-2` at 8 GB, and reading only that number +would be a mistake, because the cold writer peaked at 8,812 MiB and the cold +writer is the run that has to succeed. Size the runner for the run that +populates the cache, not the run that reads it. On that rule standard-2 is out +and `ubicloud-standard-4` at 16 GB is the safe shrink. + +The shape is unchanged here on purpose: halving the vCPU count trades wall time +against the lower rate, and a Bevy workspace is where that trade bites, so it +belongs in its own pull request with its own measurement rather than folded +into this one. The sequence is: this merge push is the cold writer on `main`, +then two sequential runs of `ci.yml` against `main` for warm evidence, then a +follow-up moving both jobs to `ubicloud-standard-4` with the samplers kept. +Accept that follow-up only if its own warm `build-test` stays under 25 minutes +and its cold writer's memory peak stays under 12 GB. `ci.yml` accepts `workflow_dispatch` so a warm run can be measured on demand. A dispatch restores what a pull request restores and writes nothing: @@ -509,48 +531,75 @@ two. A workflow contract in `tests/workflow_contracts.rs` fails if a second ### Workflow contracts -`tests/workflow_contracts.rs` asserts the rules -above: pinned cache and shared-action references, no source-built tools, one -owner per cached path, GitHub-hosted placement for non-build jobs, registered -runner labels, an installer before the first use of what it installs, and a -single test execution per build job. It also pins the inputs that make those -rules true, so a workflow cannot keep the shape of the policy while dropping -its substance: `cache-provider`, `use-sccache`, the Whitaker installer -version, the coverage flags, the uv cache paths and key, a bounded -`timeout-minutes` for each build job, the resource sampler and its report, and -the compiler-cache wiring: the two job-level variables, and the export, -install, start, build, report order that the sccache server's one-shot backend -binding depends on. +`tests/workflow_contracts.rs` asserts the rules above. It is a harness rather +than a test file: the rules live in four modules under `tests/contracts/`, +split by the question each asks. + +| Module | Asks | +| --- | --- | +| `supply_chain.rs` | What will the estate execute? Pinned cache and shared-action references, no source-built tools, prebuilt Whitaker and sccache. | +| `placement.rs` | What does it cost, and who owns each cache? Runner placement and labels, bounded timeouts, one owner per cached path, an installer before the first use of what it installs, a single test execution per build job, the uv cache key. | +| `compiler_cache.rs` | Is sccache actually working? The two job-level variables, the export, install, start, build, report order, the proxy export, and the resource sampler with its report. | +| `parsing.rs` | Does the loader read workflows correctly? Its subject is the loader, not any workflow in this repository. | + +Each module also pins the inputs that make its rules true, so a workflow cannot +keep the shape of the policy while dropping its substance: `cache-provider`, +`use-sccache`, the Whitaker installer version, the coverage flags, and the uv +cache paths and key. + +The split is not only about the 400-line limit. `parsing.rs` reads a different +subject from the other three, and separating it makes that visible: a failure +there means the loader is wrong, not that a workflow is. `tests/support/workflow_model.rs` holds the job, step, and runner-selection types the properties and the contracts share; `tests/support/workflow_estate.rs` holds the pinned commits, the whole-file `Workflow` type, and the errors parsing reports, which only the contracts need. -`tests/support/workflow_loader.rs` turns files into those values. +`tests/support/workflow_loader.rs` turns workflow files into those values, and +`tests/support/workflow_config.rs` reads the other repository files a contract +needs, currently `actionlint`'s runner registration. They are separate because +the subject differs: a failure in one is a workflow that would not parse, in +the other a configuration file that could not be read. Parsing is strict about shape and permissive about spelling. A field that is present but of the wrong type is an error rather than a silent default, because a contract that read an empty string for a mistyped `runs-on` would pass a -workflow it should reject. A step that sets both `uses` and `run` is rejected -too: GitHub Actions runs a step one way or the other, never both. Against that, -every form the platform genuinely accepts must parse. `runs-on` may be a label, -a list of labels, or a mapping naming a runner group, and `on` may be an event, -a list, or a mapping, read under the bare key that YAML 1.1 turns into the -boolean true. The files are read through a `cap_std` directory capability -rooted at `.github/workflows`. - -Action references are matched on the whole coordinate before the `@`, publisher -included. A suffix match would let `untrusted/setup-rust` satisfy a rule -written about the shared `setup-rust`, which is the opposite of what a pinning -rule is for. +workflow it should reject. The exclusive shapes GitHub Actions enforces are +enforced here too: a step sets `uses` or `run`, never both, and a job either +calls a reusable workflow or names a runner and runs its own steps, never both. +Accepting a mixture would let the contracts reason about a job the runner would +never schedule. + +Against that, every form the platform genuinely accepts must parse. `runs-on` +may be a label, a list of labels, or a mapping naming a runner group, and `on` +may be an event, a list, or a mapping, read under the bare key that YAML 1.1 +turns into the boolean true. The files are read through a `cap_std` directory +capability rooted at `.github/workflows`. + +Three matching rules exist because a loose match quietly defeats the rule it is +part of. + +- Action references are compared on the whole coordinate before the `@`, + publisher included. A suffix match would let `untrusted/setup-rust` satisfy a + rule written about the shared `setup-rust`. +- A step owns a cache only when its `uses` names `actions/cache`, + `actions/cache/restore`, or `actions/cache/save` exactly. + `actions/cache-audit` shares the prefix, caches nothing, and would otherwise + contribute an invented claim on whatever `path` input it carried. +- Runner labels are compared against the parsed + `self-hosted-runner.labels` list in `.github/actionlint.yaml`, by equality. + Searching the file as text would accept `standard-8` because + `ubicloud-standard-8` contains it, and would accept a label that appears only + in a comment. Two assurance methods are used together, following [ADR 003](adr-003-bounded-rstest-over-property-testing.md). -`tests/workflow_contracts.rs` holds bounded `rstest` cases over the workflow -files as they stand, and `tests/workflow_model_properties.rs` samples the -wider domain with `proptest`: arbitrary step orderings, repeated display -names, interleaved unrelated steps, and split caches whose halves agree or -disagree on a key, or where a third step claims a paired key. The properties +The contract modules hold bounded `rstest` cases over the workflow files as +they stand, and `tests/workflow_model_properties.rs` samples the wider domain +with `proptest`: arbitrary step orderings, repeated display names, interleaved +unrelated steps, actions that merely share the `actions/cache` prefix, and +split caches whose halves agree or disagree on a key, or where a third step +claims a paired key. The properties check cache-owner uniqueness and installer-ordering against small oracles written independently of the implementation. Run both with `make test`, and run `actionlint` after editing any workflow. diff --git a/tests/contracts/compiler_cache.rs b/tests/contracts/compiler_cache.rs new file mode 100644 index 0000000..7dd4355 --- /dev/null +++ b/tests/contracts/compiler_cache.rs @@ -0,0 +1,189 @@ +//! Compiler-cache and resource-sampling contracts. +//! +//! sccache is the only owner of compiler output here, and it fails silently +//! when it is wired wrongly: a misconfigured backend reports a plausible +//! `Cache location` and caches nothing. These contracts pin the wiring that +//! makes it work, and the sampling that lets the runner shape be argued from +//! measurement rather than habit. + +use rstest::rstest; + +use crate::shared_action; +use crate::workflow_assertions::{assert_input, job_named, step_using, workflows}; +use crate::workflow_estate::{Workflow, BUILD_JOB_IDS}; + +/// Commit that every `actions/github-script` reference must pin (v8). +const GITHUB_SCRIPT_SHA: &str = "ed597411d8f924073f98dfc5c65a23a2325f34cd"; + +/// Variables sccache's GitHub Actions backend needs re-exported on Ubicloud. +const PROXY_VARIABLES: [&str; 3] = [ + "ACTIONS_CACHE_URL", + "ACTIONS_RUNTIME_TOKEN", + "ACTIONS_CACHE_SERVICE_V2", +]; + +#[rstest] +fn setup_rust_owns_the_registry_but_not_the_compiler_cache(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let step = step_using(job, &shared_action("setup-rust")); + assert_input(id, step, "cache-provider", "github"); + // The action's sccache path runs the mozilla sccache-action, which + // writes GitHub's v2 cache service back to `GITHUB_ENV` as its last + // act, clobbering the proxy export for every later step. The job + // installs and starts sccache itself instead. + assert_input(id, step, "use-sccache", "false"); + } +} + +/// The two variables that make the wrapper more than overhead. +/// +/// `RUSTC_WRAPPER` engages sccache; `SCCACHE_GHA_ENABLED` selects the Actions +/// backend. Without the second, sccache writes to a local directory nothing +/// persists between runs, and every compilation misses. +#[rstest] +#[case::wrapper("RUSTC_WRAPPER", "sccache")] +#[case::backend("SCCACHE_GHA_ENABLED", "true")] +#[case::no_incremental("CARGO_INCREMENTAL", "0")] +fn the_compiler_cache_is_engaged_at_job_level( + workflows: Vec, + #[case] variable: &str, + #[case] expected: &str, +) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + assert_eq!( + job.env(variable), + expected, + "`{id}` must export `{variable}: {expected}` at job level" + ); + } +} + +/// The sccache server binds its backend once, when it starts, so the order of +/// these steps is the contract. Started before the export it binds GitHub's v2 +/// service instead of Ubicloud's proxy; started after `setup-rust`, whose +/// sccache path rewrites the cache service back into `GITHUB_ENV`, it binds +/// whatever that left behind; reported before the build it measures nothing. +#[rstest] +fn the_compiler_cache_is_wired_in_the_only_order_that_works(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let stage = |needle: &str, what: &str| { + job.first_step_containing(needle) + .unwrap_or_else(|| panic!("`{id}` must {what}")) + }; + let export = stage("actions/github-script", "export the Ubicloud cache proxy"); + let install = stage("taiki-e/install-action", "install a pinned sccache"); + let start = stage("sccache --zero-stats", "start the compiler cache"); + // `setup-rust` stands for the first step that could compile: it puts + // the toolchain in place, and nothing before it runs cargo. + let toolchain = stage("setup-rust", "set up Rust before anything compiles"); + let coverage = stage("generate-coverage", "build the workspace under coverage"); + let report = stage("sccache --show-stats", "report compiler-cache statistics"); + let order = [ + ("export the cache proxy", export), + ("install sccache", install), + ("start sccache", start), + ("set up the toolchain", toolchain), + ("build", coverage), + ("report the statistics", report), + ]; + for ((earlier, before), (later, after)) in order.iter().zip(order.iter().skip(1)) { + assert!( + before < after, + "`{id}` must {earlier} (step {before}) before it can {later} (step {after})" + ); + } + } +} + +#[rstest] +fn the_cache_proxy_export_is_pinned_and_names_every_variable(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let (export_at, export) = job + .first_step_with("actions/github-script") + .unwrap_or_else(|| panic!("`{id}` must export the Ubicloud cache proxy")); + assert!( + export.uses.ends_with(GITHUB_SCRIPT_SHA), + "`{id}` must pin actions/github-script to {GITHUB_SCRIPT_SHA}" + ); + let checkout_at = job + .first_step_containing("actions/checkout") + .unwrap_or_else(|| panic!("`{id}` must check out the repository")); + assert!( + checkout_at < export_at, + "`{id}` must export the proxy after checkout" + ); + let script = export.input("script"); + for variable in PROXY_VARIABLES { + assert!( + script.contains(variable), + "`{id}` must export `{variable}` for sccache's backend" + ); + } + assert!( + !script.contains("ACTIONS_RESULTS_URL"), + "`{id}` must not export ACTIONS_RESULTS_URL; it does not route \ + through Ubicloud's cache proxy" + ); + } +} + +#[rstest] +fn compiler_cache_effectiveness_is_measured_around_the_build(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let zero_at = job + .first_step_containing("sccache --zero-stats") + .unwrap_or_else(|| panic!("`{id}` must reset the compiler-cache counters")); + let (show_at, report) = job + .first_step_with("sccache --show-stats") + .unwrap_or_else(|| panic!("`{id}` must report compiler-cache statistics")); + assert!( + zero_at < show_at, + "`{id}` must reset the counters before it reports them" + ); + assert!( + report.run.contains("GITHUB_STEP_SUMMARY"), + "`{id}` must put the compiler-cache statistics in the job summary" + ); + // The summary is not readable through the REST API, so a run whose + // statistics went only there cannot be audited afterwards. + assert!( + report.run.contains("printf '%s\\n' \"$stats\""), + "`{id}` must also print the compiler-cache statistics to the log" + ); + } +} + +/// The `ubicloud-standard-8` shape is inherited here, not measured. Sampling +/// memory and disk is what turns the next shape decision into evidence, and +/// disk is the one that has killed jobs silently elsewhere in this rollout. +#[rstest] +fn both_build_jobs_sample_and_report_their_resource_use(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let start = job + .first_step_containing("sample-resources.sh") + .unwrap_or_else(|| panic!("`{id}` must start a resource sampler")); + let (report_at, report) = job + .first_step_with("least free disk") + .unwrap_or_else(|| panic!("`{id}` must report its peak resource use")); + assert!( + start < report_at, + "`{id}` must start the sampler before it reports the peaks" + ); + for measure in ["free -m", "df -m"] { + assert!( + job.steps.iter().any(|step| step.run.contains(measure)), + "`{id}` must sample `{measure}`; disk and memory are both needed" + ); + } + assert!( + report.run.contains("peak used disk"), + "`{id}` must report peak disk, not memory alone" + ); + } +} diff --git a/tests/contracts/parsing.rs b/tests/contracts/parsing.rs new file mode 100644 index 0000000..cf7621b --- /dev/null +++ b/tests/contracts/parsing.rs @@ -0,0 +1,90 @@ +//! Parser contracts over the workflow loader. +//! +//! These read no workflow file in the repository. Their subject is the loader +//! itself: that it rejects a document whose shape the runner would reject, and +//! accepts every shape the runner accepts. A loader that silently defaulted a +//! mistyped field would let a broken workflow satisfy every rule in the other +//! contract modules. + +use camino::Utf8Path; +use rstest::rstest; + +use crate::workflow_estate::WorkflowSource; +use crate::workflow_loader::{load_workflows_in, parse_workflow}; + +#[rstest] +#[case::not_a_workflow("scratch.yml", "steps: []")] +#[case::mistyped_runner("scratch.yml", "jobs:\n a:\n runs-on: {group: [g]}\n")] +#[case::mistyped_runner_label("scratch.yml", "jobs:\n a:\n runs-on: [a, [b]]\n")] +#[case::groupless_runner_mapping("scratch.yml", "jobs:\n a:\n runs-on: {labels: [a]}\n")] +#[case::placeless_job("scratch.yml", "jobs:\n a:\n steps: []\n")] +#[case::mistyped_steps("scratch.yml", "jobs:\n a:\n runs-on: x\n steps: nope\n")] +#[case::empty_step( + "scratch.yml", + "jobs:\n a:\n runs-on: x\n steps:\n - name: n\n" +)] +#[case::mistyped_input( + "scratch.yml", + "jobs:\n a:\n runs-on: x\n steps:\n - uses: u\n with:\n k: [1]\n" +)] +// GitHub Actions runs a step either as an action or as a script, never both. +#[case::dual_mode_step( + "scratch.yml", + "jobs:\n a:\n runs-on: x\n steps:\n - uses: u\n run: echo hi\n" +)] +// A job calls a reusable workflow or runs its own steps on a runner it names. +// GitHub Actions rejects either mixture. +#[case::reusable_job_with_a_runner( + "scratch.yml", + "jobs:\n a:\n uses: o/r/.github/workflows/w.yml@v1\n runs-on: x\n" +)] +#[case::reusable_job_with_steps( + "scratch.yml", + "jobs:\n a:\n uses: o/r/.github/workflows/w.yml@v1\n steps: []\n" +)] +fn a_malformed_workflow_is_an_error_not_a_default(#[case] file: &str, #[case] text: &str) { + let outcome = parse_workflow(WorkflowSource { file, text }); + assert!( + outcome.is_err(), + "a workflow of unexpected shape must be rejected, not silently defaulted" + ); +} + +/// Every `runs-on` shape GitHub Actions accepts must parse, not just the +/// scalar one: rejecting a label list or a runner group would fail a valid +/// workflow rather than the workflow a contract is meant to catch. +#[rstest] +#[case::single_label("runs-on: ubuntu-latest\n", &["ubuntu-latest"])] +#[case::label_list("runs-on: [self-hosted, linux]\n", &["self-hosted", "linux"])] +#[case::group_only("runs-on:\n group: ubuntu-runners\n", &[])] +#[case::group_and_labels( + "runs-on:\n group: ubuntu-runners\n labels: [ubuntu-20.04-16core]\n", + &["ubuntu-20.04-16core"] +)] +fn every_valid_runs_on_shape_parses(#[case] runs_on: &str, #[case] expected: &[&str]) { + let text = format!("on: push\njobs:\n a:\n {runs_on} steps: []\n"); + let workflow = parse_workflow(WorkflowSource { + file: "scratch.yml", + text: &text, + }) + .unwrap_or_else(|err| panic!("`{runs_on}` must parse: {err}")); + let job = workflow + .jobs + .first() + .unwrap_or_else(|| panic!("`{runs_on}` must yield a job")); + assert_eq!(job.runs_on.labels(), expected); + assert!( + job.runs_on.names_a_runner(), + "`{runs_on}` names a runner and must say so" + ); +} + +#[rstest] +fn an_unreadable_workflow_directory_is_reported() { + let missing = Utf8Path::new("this/directory/does/not/exist"); + let outcome = load_workflows_in(missing); + assert!( + outcome.is_err(), + "an unreadable workflow directory must surface as an error" + ); +} diff --git a/tests/contracts/placement.rs b/tests/contracts/placement.rs new file mode 100644 index 0000000..ed0b173 --- /dev/null +++ b/tests/contracts/placement.rs @@ -0,0 +1,197 @@ +//! Placement, cache-ownership, and job-shape contracts. +//! +//! Which runner a job uses, what it is allowed to bill, who owns each cached +//! path, and that the suite runs once rather than twice. These are the rules +//! that decide what the estate costs and whether a cache miss is explainable. + +use rstest::rstest; + +use crate::shared_action; +use crate::workflow_assertions::{assert_input, job_named, jobs, step_using, workflows}; +use crate::workflow_cache_owners; +use crate::workflow_config::registered_runner_labels; +use crate::workflow_estate::{Workflow, BUILD_JOB_IDS, UBICLOUD_LABEL}; +use crate::workflow_loader::all_steps; + +/// Commands that would run the test suite a second time in a build job. +const REPEAT_TEST_COMMANDS: [&str; 4] = ["cargo test", "cargo nextest", "make test", "make all"]; + +/// Expression fragments the uv tool-layer cache key must carry. +const UV_CACHE_KEY_FRAGMENTS: [&str; 4] = [ + "runner.os", + "runner.arch", + "runner.environment", + "hashFiles(", +]; + +#[rstest] +fn each_cached_path_has_exactly_one_owner(workflows: Vec) { + let clashes: Vec = jobs(&workflows) + .into_iter() + .flat_map(|(file, job)| { + workflow_cache_owners::duplicated_paths(&job) + .into_iter() + .map(move |(path, owners)| format!("{file}:{}: {path} owned by {owners:?}", job.id)) + }) + .collect(); + assert!( + clashes.is_empty(), + "each cached path must have one owner: {clashes:?}" + ); +} + +#[rstest] +fn non_build_jobs_stay_on_github_hosted_runners(workflows: Vec) { + let misplaced: Vec = jobs(&workflows) + .into_iter() + .filter(|(_, job)| job.runs_on.names_a_runner()) + .filter(|(_, job)| !BUILD_JOB_IDS.contains(&job.id.as_str())) + .filter(|(_, job)| !job.is_github_hosted()) + .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) + .collect(); + assert!( + misplaced.is_empty(), + "delayed-comment, metadata, and other API-bound jobs must stay GitHub-hosted: {misplaced:?}" + ); +} + +/// The measured bounds for each build job's `timeout-minutes`. +/// +/// The lower bound keeps the timeout above the observed median so a normal run +/// cannot be killed; the upper bound keeps a hung run from billing for hours. +#[rstest] +#[case::build_test("build-test", 45, 120)] +#[case::coverage_upload("coverage-upload", 30, 90)] +fn build_jobs_keep_their_label_and_a_bounded_timeout( + workflows: Vec, + #[case] id: &str, + #[case] lowest: u64, + #[case] highest: u64, +) { + let job = job_named(&workflows, id); + assert_eq!( + job.runs_on.labels(), + [UBICLOUD_LABEL], + "`{id}` must keep its measured runner label" + ); + let timeout = job + .timeout_minutes + .unwrap_or_else(|| panic!("`{id}` bills by the minute and must declare timeout-minutes")); + assert!( + (lowest..=highest).contains(&timeout), + "`{id}` timeout-minutes {timeout} must lie between {lowest} and {highest}" + ); +} + +/// A warm run has to be triggerable without pushing a commit, so the runner +/// and cache changes can be measured on an unchanged tree. +#[rstest] +fn the_pull_request_workflow_accepts_a_warm_run_dispatch(workflows: Vec) { + let Some(ci) = workflows.iter().find(|workflow| workflow.file == "ci.yml") else { + panic!("the estate must define ci.yml") + }; + assert!( + ci.has_trigger("workflow_dispatch"), + "ci.yml must accept `workflow_dispatch` so a warm run can be measured \ + on demand; it declares {:?}", + ci.triggers + ); +} + +#[rstest] +fn every_runner_label_is_registered_with_actionlint(workflows: Vec) { + let registered = registered_runner_labels() + .unwrap_or_else(|err| panic!("actionlint configuration must be readable: {err}")); + let unregistered: Vec = jobs(&workflows) + .into_iter() + .filter(|(_, job)| job.runs_on.names_a_runner() && !job.is_github_hosted()) + .filter(|(_, job)| { + !job.runs_on + .labels() + .iter() + .all(|label| registered.contains(label)) + }) + .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) + .collect(); + assert!( + unregistered.is_empty(), + "every self-hosted label must appear in .github/actionlint.yaml: {unregistered:?}" + ); +} + +#[rstest] +#[case::rust_toolchain("setup-rust", "cargo")] +#[case::whitaker_suite("install-whitaker", "whitaker ")] +fn an_installer_precedes_the_first_use_of_its_tool( + workflows: Vec, + #[case] installer: &str, + #[case] first_use: &str, +) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let Some(use_index) = job.first_step_containing(first_use) else { + continue; + }; + let install_index = job + .first_step_containing(installer) + .unwrap_or_else(|| panic!("`{id}` uses `{first_use}` without a `{installer}` step")); + assert!( + install_index < use_index, + "`{id}` must run `{installer}` before step {use_index} uses `{first_use}`" + ); + } +} + +#[rstest] +fn coverage_is_the_only_test_execution(workflows: Vec) { + let duplicates: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, job, _)| BUILD_JOB_IDS.contains(&job.as_str())) + .filter(|(_, _, step)| { + REPEAT_TEST_COMMANDS + .iter() + .any(|command| step.run.contains(command)) + }) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.label())) + .collect(); + assert!( + duplicates.is_empty(), + "the instrumented coverage run is the only test execution; drop the repeat: {duplicates:?}" + ); +} + +#[rstest] +fn coverage_runs_the_whole_suite_once_and_owns_no_cargo_cache(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let step = step_using(job, &shared_action("generate-coverage")); + for flag in ["all-features", "all-targets", "doctests"] { + assert_input(id, step, flag, "true"); + } + assert_input(id, step, "cache-provider", "external"); + } +} + +#[rstest] +fn the_uv_cache_names_its_layers_and_keys_them_by_runner(workflows: Vec) { + let job = job_named(&workflows, "build-test"); + let cache = job + .steps + .iter() + .find(|step| step.cache_paths().iter().any(|path| path == ".uv-cache")); + let Some(step) = cache else { + panic!("`build-test` must cache the uv download layer") + }; + assert_eq!( + step.cache_paths(), + vec![".uv-cache".to_owned(), ".uv-tools".to_owned()], + "the uv cache must own both the download store and the tool store" + ); + let key = step.input("key"); + for fragment in UV_CACHE_KEY_FRAGMENTS { + assert!( + key.contains(fragment), + "the uv cache key `{key}` must vary with `{fragment}`" + ); + } +} diff --git a/tests/contracts/supply_chain.rs b/tests/contracts/supply_chain.rs new file mode 100644 index 0000000..7721264 --- /dev/null +++ b/tests/contracts/supply_chain.rs @@ -0,0 +1,124 @@ +//! Supply-chain contracts over the workflow estate. +//! +//! Every third-party reference is pinned to a commit, and every tool arrives +//! as a verified prebuilt release. These are the rules that decide what code +//! the estate is willing to execute, so a violation is a trust question rather +//! than a performance one. + +use rstest::rstest; + +use crate::shared_action; +use crate::workflow_assertions::{assert_input, job_named, jobs, step_using, workflows}; +use crate::workflow_cache_owners::is_cache_action; +use crate::workflow_estate::{Workflow, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_SHA}; +use crate::workflow_loader::all_steps; + +/// Fragments that mark a step as building a tool from source. +/// +/// `cargo binstall` is included because it compiles whenever its default +/// strategies fall through to `compile`; the estate's rule is to install from +/// a verified release archive instead. +const SOURCE_BUILD_FRAGMENTS: [&str; 3] = ["cargo install", "cargo-binstall ", "cargo binstall"]; + +/// Pinned prebuilt sccache the build jobs install. +const SCCACHE_TOOL: &str = "sccache@0.16.0"; + +#[rstest] +fn every_cache_reference_is_pinned_to_v6_1_0(workflows: Vec) { + let unpinned: Vec = all_steps(&workflows) + .into_iter() + // Matched on the exact coordinate: `actions/cache-audit` shares the + // prefix and is a different action, which this rule has nothing to say + // about. + .filter(|(_, _, step)| is_cache_action(&step.uses)) + .filter(|(_, _, step)| !step.uses.ends_with(CACHE_ACTION_SHA)) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.uses)) + .collect(); + assert!( + unpinned.is_empty(), + "every actions/cache reference must pin {CACHE_ACTION_SHA} (v6.1.0): {unpinned:?}" + ); +} + +#[rstest] +fn no_workflow_uses_the_ubicloud_cache_fork(workflows: Vec) { + let forks: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, _, step)| step.uses.starts_with("ubicloud/cache")) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.uses)) + .collect(); + assert!( + forks.is_empty(), + "the deprecated ubicloud/cache fork must not be used: {forks:?}" + ); +} + +#[rstest] +fn every_shared_action_reference_is_pinned(workflows: Vec) { + let mut references: Vec = all_steps(&workflows) + .into_iter() + .map(|(file, job, step)| (file, job, step.uses)) + .chain( + jobs(&workflows) + .into_iter() + .map(|(file, job)| (file, job.id.clone(), job.uses)), + ) + .filter(|(_, _, uses)| uses.starts_with("leynos/shared-actions")) + .filter(|(_, _, uses)| !uses.ends_with(SHARED_ACTIONS_SHA)) + .map(|(file, job, uses)| format!("{file}:{job}: {uses}")) + .collect(); + references.sort(); + assert!( + references.is_empty(), + "every leynos/shared-actions reference must pin {SHARED_ACTIONS_SHA}: {references:?}" + ); +} + +#[rstest] +fn no_step_builds_a_tool_from_source(workflows: Vec) { + let offenders: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, _, step)| { + SOURCE_BUILD_FRAGMENTS + .iter() + .any(|fragment| step.run.contains(fragment)) + }) + .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) + .collect(); + assert!( + offenders.is_empty(), + "tools must be installed from verified prebuilt releases, not compiled: {offenders:?}" + ); +} + +#[rstest] +fn install_action_fails_closed_rather_than_compiling(workflows: Vec) { + let permissive: Vec = all_steps(&workflows) + .into_iter() + .filter(|(_, _, step)| step.uses.starts_with("taiki-e/install-action")) + .filter(|(_, _, step)| step.input("fallback") != "none") + .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) + .collect(); + assert!( + permissive.is_empty(), + "taiki-e/install-action must set `fallback: none` so it cannot compile: {permissive:?}" + ); +} + +#[rstest] +fn sccache_is_installed_from_a_pinned_prebuilt_release(workflows: Vec) { + for id in BUILD_JOB_IDS { + let job = job_named(&workflows, id); + let step = step_using(job, "taiki-e/install-action"); + assert_input(id, step, "tool", SCCACHE_TOOL); + assert_input(id, step, "fallback", "none"); + } +} + +#[rstest] +fn whitaker_is_installed_from_a_pinned_prebuilt_release(workflows: Vec) { + let job = job_named(&workflows, "build-test"); + let step = step_using(job, &shared_action("install-whitaker")); + assert_input("build-test", step, "installer-version", "0.2.7"); + assert_input("build-test", step, "cache-provider", "github"); +} diff --git a/tests/support/workflow_cache_owners.rs b/tests/support/workflow_cache_owners.rs index 834dc9f..61a170e 100644 --- a/tests/support/workflow_cache_owners.rs +++ b/tests/support/workflow_cache_owners.rs @@ -6,6 +6,10 @@ //! the path. This module reduces both kinds to the same `(path, owner)` list //! so one contract can compare them. //! +//! A step owns a cache only when its `uses` names one of the three +//! `actions/cache` coordinates exactly. A prefix match would enrol +//! `actions/cache-audit` and invent a claim it never makes. +//! //! Owner identity is the step's position in its job, never its display name: //! two steps may legitimately share a name, and collapsing them would hide a //! duplicate owner. The one deliberate exception is a split cache, where an @@ -71,6 +75,23 @@ fn action_path(uses: &str) -> &str { uses.split('@').next().unwrap_or_default() } +/// The three `actions/cache` coordinates that make a step a cache owner. +const CACHE_ACTIONS: [&str; 3] = [ + "actions/cache", + "actions/cache/restore", + "actions/cache/save", +]; + +/// Reports whether a `uses` reference is one of the cache actions. +/// +/// Matched exactly rather than by prefix: `actions/cache-audit` shares the +/// prefix but caches nothing, and treating it as an owner would invent a +/// duplicate claim on whatever path it happened to carry. +#[must_use] +pub fn is_cache_action(uses: &str) -> bool { + CACHE_ACTIONS.contains(&action_path(uses)) +} + /// The half of a split cache a step is, if it is one. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum SplitHalf { @@ -115,7 +136,7 @@ fn owner_identity(step: &Step, index: usize, paired: bool) -> String { /// Returns the claims an `actions/cache` step makes on its own `path` input. fn direct_owners(step: &Step, index: usize, paired: bool) -> Vec { - if !action_path(&step.uses).starts_with("actions/cache") { + if !is_cache_action(&step.uses) { return Vec::new(); } let owner = owner_identity(step, index, paired); diff --git a/tests/support/workflow_config.rs b/tests/support/workflow_config.rs new file mode 100644 index 0000000..ab386ac --- /dev/null +++ b/tests/support/workflow_config.rs @@ -0,0 +1,72 @@ +//! Repository configuration this estate's contracts read. +//! +//! `workflow_loader.rs` reads `.github/workflows`. This module reads the other +//! repository files a contract needs, currently only `actionlint`'s runner +//! registration. Kept apart because the subject differs: a failure here is a +//! configuration file the contracts could not read, not a workflow they could +//! not parse. +//! +//! Files are read through a `cap_std` directory capability rooted at the +//! repository, so this module cannot reach outside it. +//! +//! # Examples +//! +//! ```no_run +//! let labels = workflow_config::registered_runner_labels()?; +//! assert!(labels.iter().all(|label| !label.is_empty())); +//! # Ok::<(), workflow_estate::WorkflowError>(()) +//! ``` + +use camino::Utf8PathBuf; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use serde_norway::Value; + +use crate::workflow_estate::{Location, WorkflowError}; +use crate::workflow_loader::render_scalar; + +/// Reads a file from this repository's root through a directory capability. +/// +/// # Errors +/// +/// Returns an error when the repository root cannot be opened or the file +/// cannot be read. +fn read_repository_file(relative: &str) -> Result { + let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let dir = Dir::open_ambient_dir(&root, ambient_authority()) + .map_err(|err| WorkflowError::Read(root.to_string(), err))?; + dir.read_to_string(relative) + .map_err(|err| WorkflowError::Read(relative.to_owned(), err)) +} + +/// Reads the self-hosted runner labels `actionlint` is configured to accept. +/// +/// Parsed rather than searched as text: a substring test would accept +/// `standard-8` because `ubicloud-standard-8` contains it, and would accept a +/// label that appears only in a comment. The contract exists to prove a label +/// is registered, so it has to compare whole entries. +/// +/// # Errors +/// +/// Returns an error when the file cannot be read or is not a mapping whose +/// `self-hosted-runner.labels` is a sequence of strings. +pub fn registered_runner_labels() -> Result, WorkflowError> { + const FILE: &str = ".github/actionlint.yaml"; + let at = Location::file(FILE); + let text = read_repository_file(FILE)?; + let document: Value = + serde_norway::from_str(&text).map_err(|err| WorkflowError::Parse(FILE.to_owned(), err))?; + let Some(labels) = document + .get("self-hosted-runner") + .and_then(|it| it.get("labels")) + else { + return Ok(Vec::new()); + }; + labels + .as_sequence() + .ok_or_else(|| at.shape("`self-hosted-runner.labels` must be a sequence"))? + .iter() + .map(|label| { + render_scalar(label).ok_or_else(|| at.shape("every registered label must be a scalar")) + }) + .collect() +} diff --git a/tests/support/workflow_loader.rs b/tests/support/workflow_loader.rs index d7091f5..b9486cf 100644 --- a/tests/support/workflow_loader.rs +++ b/tests/support/workflow_loader.rs @@ -31,7 +31,7 @@ use crate::workflow_model::{Job, RunnerSelection, Step}; /// GitHub Actions coerces booleans and numbers to strings when it passes an /// input to an action, so `doctests: true` and `doctests: 'true'` reach the /// action identically and must compare equal here too. -fn render_scalar(value: &Value) -> Option { +pub fn render_scalar(value: &Value) -> Option { match value { Value::String(text) => Some(text.clone()), Value::Bool(flag) => Some(flag.to_string()), @@ -172,14 +172,28 @@ fn parse_runs_on(raw: &Value, at: &Location) -> Result bool { + if job.uses.is_empty() { + return false; + } + job.runs_on.names_a_runner() || declares_steps +} + /// Parses one job of a workflow. /// /// # Errors /// -/// Returns an error when a field is mistyped, `steps` is not a sequence, or -/// the job neither names a runner nor calls a reusable workflow. +/// Returns an error when a field is mistyped, `steps` is not a sequence, the +/// job neither names a runner nor calls a reusable workflow, or it calls a +/// reusable workflow and also sets `runs-on` or `steps`. fn parse_job(id: &str, raw: &Value, file: &Location) -> Result { let at = file.job(id); + let declares_steps = raw.get("steps").is_some(); let steps = match raw.get("steps") { None => Vec::new(), Some(value) => value @@ -200,6 +214,11 @@ fn parse_job(id: &str, raw: &Value, file: &Location) -> Result Result, WorkflowError> { load_workflows_in(&root) } -/// Reads a file from this repository's root through a directory capability. -/// -/// # Errors -/// -/// Returns an error when the repository root cannot be opened or the file -/// cannot be read. -pub fn read_repository_file(relative: &str) -> Result { - let root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")); - let dir = Dir::open_ambient_dir(&root, ambient_authority()) - .map_err(|err| WorkflowError::Read(root.to_string(), err))?; - dir.read_to_string(relative) - .map_err(|err| WorkflowError::Read(relative.to_owned(), err)) -} - /// Returns every step of every job, tagged with its workflow and job. #[must_use] pub fn all_steps(workflows: &[Workflow]) -> Vec<(String, String, Step)> { diff --git a/tests/workflow_contracts.rs b/tests/workflow_contracts.rs index 5bddf57..d66dbc8 100644 --- a/tests/workflow_contracts.rs +++ b/tests/workflow_contracts.rs @@ -1,18 +1,22 @@ //! Structural contracts over the repository's GitHub Actions workflows. //! //! These encode the Ubicloud adoption rules a reviewer would otherwise re-check -//! by hand on every workflow edit: no tool is built from source, every cached -//! path has one owner, references are pinned, API-bound jobs stay -//! GitHub-hosted, and an installer precedes the first use of what it installs. -//! They also pin the inputs that make those rules true, so a workflow cannot -//! keep the shape of the policy while dropping its substance. They read the -//! files directly, so they fail on the change that introduces a violation -//! rather than on the CI run that suffers from it. +//! by hand on every workflow edit. They read the files directly, so they fail +//! on the change that introduces a violation rather than on the CI run that +//! suffers from it. +//! +//! This file is the harness. The rules live in four modules, split by the +//! question each asks: `supply_chain` for what the estate will execute, +//! `placement` for what it costs and who owns each cache, `compiler_cache` for +//! the sccache wiring and the resource sampling, and `parsing` for the loader +//! itself. #[path = "support/workflow_assertions.rs"] mod workflow_assertions; #[path = "support/workflow_cache_owners.rs"] mod workflow_cache_owners; +#[path = "support/workflow_config.rs"] +mod workflow_config; #[path = "support/workflow_estate.rs"] mod workflow_estate; #[path = "support/workflow_loader.rs"] @@ -20,550 +24,19 @@ mod workflow_loader; #[path = "support/workflow_model.rs"] mod workflow_model; -use camino::Utf8Path; -use rstest::rstest; +#[path = "contracts/compiler_cache.rs"] +mod compiler_cache; +#[path = "contracts/parsing.rs"] +mod parsing; +#[path = "contracts/placement.rs"] +mod placement; +#[path = "contracts/supply_chain.rs"] +mod supply_chain; -use workflow_assertions::{assert_input, job_named, jobs, step_using, workflows}; -use workflow_estate::{ - Workflow, WorkflowSource, BUILD_JOB_IDS, CACHE_ACTION_SHA, SHARED_ACTIONS_OWNER, - SHARED_ACTIONS_SHA, UBICLOUD_LABEL, -}; -use workflow_loader::{all_steps, load_workflows_in, parse_workflow, read_repository_file}; +use workflow_estate::SHARED_ACTIONS_OWNER; /// Full coordinate of a shared composite action this repository calls. -fn shared_action(name: &str) -> String { +#[must_use] +pub fn shared_action(name: &str) -> String { format!("{SHARED_ACTIONS_OWNER}/.github/actions/{name}") } - -/// Fragments that mark a step as building a tool from source. -/// -/// `cargo binstall` is included because it compiles whenever its default -/// strategies fall through to `compile`; the estate's rule is to install from -/// a verified release archive instead. -const SOURCE_BUILD_FRAGMENTS: [&str; 3] = ["cargo install", "cargo-binstall ", "cargo binstall"]; - -/// Commands that would run the test suite a second time in a build job. -const REPEAT_TEST_COMMANDS: [&str; 4] = ["cargo test", "cargo nextest", "make test", "make all"]; - -/// Commit that every `actions/github-script` reference must pin (v8). -const GITHUB_SCRIPT_SHA: &str = "ed597411d8f924073f98dfc5c65a23a2325f34cd"; - -/// Pinned prebuilt sccache the build jobs install. -const SCCACHE_TOOL: &str = "sccache@0.16.0"; - -/// Variables sccache's GitHub Actions backend needs re-exported on Ubicloud. -const PROXY_VARIABLES: [&str; 3] = [ - "ACTIONS_CACHE_URL", - "ACTIONS_RUNTIME_TOKEN", - "ACTIONS_CACHE_SERVICE_V2", -]; - -/// Expression fragments the uv tool-layer cache key must carry. -const UV_CACHE_KEY_FRAGMENTS: [&str; 4] = [ - "runner.os", - "runner.arch", - "runner.environment", - "hashFiles(", -]; - -#[rstest] -fn every_cache_reference_is_pinned_to_v6_1_0(workflows: Vec) { - let unpinned: Vec = all_steps(&workflows) - .into_iter() - .filter(|(_, _, step)| step.uses.starts_with("actions/cache")) - .filter(|(_, _, step)| !step.uses.ends_with(CACHE_ACTION_SHA)) - .map(|(file, job, step)| format!("{file}:{job}: {}", step.uses)) - .collect(); - assert!( - unpinned.is_empty(), - "every actions/cache reference must pin {CACHE_ACTION_SHA} (v6.1.0): {unpinned:?}" - ); -} - -#[rstest] -fn no_workflow_uses_the_ubicloud_cache_fork(workflows: Vec) { - let forks: Vec = all_steps(&workflows) - .into_iter() - .filter(|(_, _, step)| step.uses.starts_with("ubicloud/cache")) - .map(|(file, job, step)| format!("{file}:{job}: {}", step.uses)) - .collect(); - assert!( - forks.is_empty(), - "the deprecated ubicloud/cache fork must not be used: {forks:?}" - ); -} - -#[rstest] -fn every_shared_action_reference_is_pinned(workflows: Vec) { - let mut references: Vec = all_steps(&workflows) - .into_iter() - .map(|(file, job, step)| (file, job, step.uses)) - .chain( - jobs(&workflows) - .into_iter() - .map(|(file, job)| (file, job.id.clone(), job.uses)), - ) - .filter(|(_, _, uses)| uses.starts_with("leynos/shared-actions")) - .filter(|(_, _, uses)| !uses.ends_with(SHARED_ACTIONS_SHA)) - .map(|(file, job, uses)| format!("{file}:{job}: {uses}")) - .collect(); - references.sort(); - assert!( - references.is_empty(), - "every leynos/shared-actions reference must pin {SHARED_ACTIONS_SHA}: {references:?}" - ); -} - -#[rstest] -fn no_step_builds_a_tool_from_source(workflows: Vec) { - let offenders: Vec = all_steps(&workflows) - .into_iter() - .filter(|(_, _, step)| { - SOURCE_BUILD_FRAGMENTS - .iter() - .any(|fragment| step.run.contains(fragment)) - }) - .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) - .collect(); - assert!( - offenders.is_empty(), - "tools must be installed from verified prebuilt releases, not compiled: {offenders:?}" - ); -} - -#[rstest] -fn install_action_fails_closed_rather_than_compiling(workflows: Vec) { - let permissive: Vec = all_steps(&workflows) - .into_iter() - .filter(|(_, _, step)| step.uses.starts_with("taiki-e/install-action")) - .filter(|(_, _, step)| step.input("fallback") != "none") - .map(|(file, job, step)| format!("{file}:{job}: {}", step.name)) - .collect(); - assert!( - permissive.is_empty(), - "taiki-e/install-action must set `fallback: none` so it cannot compile: {permissive:?}" - ); -} - -#[rstest] -fn each_cached_path_has_exactly_one_owner(workflows: Vec) { - let clashes: Vec = jobs(&workflows) - .into_iter() - .flat_map(|(file, job)| { - workflow_cache_owners::duplicated_paths(&job) - .into_iter() - .map(move |(path, owners)| format!("{file}:{}: {path} owned by {owners:?}", job.id)) - }) - .collect(); - assert!( - clashes.is_empty(), - "each cached path must have one owner: {clashes:?}" - ); -} - -#[rstest] -fn non_build_jobs_stay_on_github_hosted_runners(workflows: Vec) { - let misplaced: Vec = jobs(&workflows) - .into_iter() - .filter(|(_, job)| job.runs_on.names_a_runner()) - .filter(|(_, job)| !BUILD_JOB_IDS.contains(&job.id.as_str())) - .filter(|(_, job)| !job.is_github_hosted()) - .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) - .collect(); - assert!( - misplaced.is_empty(), - "delayed-comment, metadata, and other API-bound jobs must stay GitHub-hosted: {misplaced:?}" - ); -} - -/// The measured bounds for each build job's `timeout-minutes`. -/// -/// The lower bound keeps the timeout above the observed median so a normal run -/// cannot be killed; the upper bound keeps a hung run from billing for hours. -#[rstest] -#[case::build_test("build-test", 45, 120)] -#[case::coverage_upload("coverage-upload", 30, 90)] -fn build_jobs_keep_their_label_and_a_bounded_timeout( - workflows: Vec, - #[case] id: &str, - #[case] lowest: u64, - #[case] highest: u64, -) { - let job = job_named(&workflows, id); - assert_eq!( - job.runs_on.labels(), - [UBICLOUD_LABEL], - "`{id}` must keep its measured runner label" - ); - let timeout = job - .timeout_minutes - .unwrap_or_else(|| panic!("`{id}` bills by the minute and must declare timeout-minutes")); - assert!( - (lowest..=highest).contains(&timeout), - "`{id}` timeout-minutes {timeout} must lie between {lowest} and {highest}" - ); -} - -/// A warm run has to be triggerable without pushing a commit, so the runner -/// and cache changes can be measured on an unchanged tree. -#[rstest] -fn the_pull_request_workflow_accepts_a_warm_run_dispatch(workflows: Vec) { - let Some(ci) = workflows.iter().find(|workflow| workflow.file == "ci.yml") else { - panic!("the estate must define ci.yml") - }; - assert!( - ci.has_trigger("workflow_dispatch"), - "ci.yml must accept `workflow_dispatch` so a warm run can be measured \ - on demand; it declares {:?}", - ci.triggers - ); -} - -#[rstest] -fn every_runner_label_is_registered_with_actionlint(workflows: Vec) { - let text = read_repository_file(".github/actionlint.yaml") - .unwrap_or_else(|err| panic!("actionlint configuration must be readable: {err}")); - let unregistered: Vec = jobs(&workflows) - .into_iter() - .filter(|(_, job)| job.runs_on.names_a_runner() && !job.is_github_hosted()) - .filter(|(_, job)| { - !job.runs_on - .labels() - .iter() - .all(|label| text.contains(label.as_str())) - }) - .map(|(file, job)| format!("{file}:{}: {}", job.id, job.runs_on)) - .collect(); - assert!( - unregistered.is_empty(), - "every self-hosted label must appear in .github/actionlint.yaml: {unregistered:?}" - ); -} - -#[rstest] -#[case::rust_toolchain("setup-rust", "cargo")] -#[case::whitaker_suite("install-whitaker", "whitaker ")] -fn an_installer_precedes_the_first_use_of_its_tool( - workflows: Vec, - #[case] installer: &str, - #[case] first_use: &str, -) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let Some(use_index) = job.first_step_containing(first_use) else { - continue; - }; - let install_index = job - .first_step_containing(installer) - .unwrap_or_else(|| panic!("`{id}` uses `{first_use}` without a `{installer}` step")); - assert!( - install_index < use_index, - "`{id}` must run `{installer}` before step {use_index} uses `{first_use}`" - ); - } -} - -#[rstest] -fn coverage_is_the_only_test_execution(workflows: Vec) { - let duplicates: Vec = all_steps(&workflows) - .into_iter() - .filter(|(_, job, _)| BUILD_JOB_IDS.contains(&job.as_str())) - .filter(|(_, _, step)| { - REPEAT_TEST_COMMANDS - .iter() - .any(|command| step.run.contains(command)) - }) - .map(|(file, job, step)| format!("{file}:{job}: {}", step.label())) - .collect(); - assert!( - duplicates.is_empty(), - "the instrumented coverage run is the only test execution; drop the repeat: {duplicates:?}" - ); -} - -#[rstest] -fn setup_rust_owns_the_registry_but_not_the_compiler_cache(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let step = step_using(job, &shared_action("setup-rust")); - assert_input(id, step, "cache-provider", "github"); - // The action's sccache path runs the mozilla sccache-action, which - // writes GitHub's v2 cache service back to `GITHUB_ENV` as its last - // act, clobbering the proxy export for every later step. The job - // installs and starts sccache itself instead. - assert_input(id, step, "use-sccache", "false"); - } -} - -/// The two variables that make the wrapper more than overhead. -/// -/// `RUSTC_WRAPPER` engages sccache; `SCCACHE_GHA_ENABLED` selects the Actions -/// backend. Without the second, sccache writes to a local directory nothing -/// persists between runs, and every compilation misses. -#[rstest] -#[case::wrapper("RUSTC_WRAPPER", "sccache")] -#[case::backend("SCCACHE_GHA_ENABLED", "true")] -#[case::no_incremental("CARGO_INCREMENTAL", "0")] -fn the_compiler_cache_is_engaged_at_job_level( - workflows: Vec, - #[case] variable: &str, - #[case] expected: &str, -) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - assert_eq!( - job.env(variable), - expected, - "`{id}` must export `{variable}: {expected}` at job level" - ); - } -} - -#[rstest] -fn sccache_is_installed_from_a_pinned_prebuilt_release(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let step = step_using(job, "taiki-e/install-action"); - assert_input(id, step, "tool", SCCACHE_TOOL); - assert_input(id, step, "fallback", "none"); - } -} - -/// The sccache server binds its backend once, when it starts, so the order of -/// these steps is the contract. Started before the export it binds GitHub's v2 -/// service instead of Ubicloud's proxy; started after `setup-rust`, whose -/// sccache path rewrites the cache service back into `GITHUB_ENV`, it binds -/// whatever that left behind; reported before the build it measures nothing. -#[rstest] -fn the_compiler_cache_is_wired_in_the_only_order_that_works(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let stage = |needle: &str, what: &str| { - job.first_step_containing(needle) - .unwrap_or_else(|| panic!("`{id}` must {what}")) - }; - let export = stage("actions/github-script", "export the Ubicloud cache proxy"); - let install = stage("taiki-e/install-action", "install a pinned sccache"); - let start = stage("sccache --zero-stats", "start the compiler cache"); - // `setup-rust` stands for the first step that could compile: it puts - // the toolchain in place, and nothing before it runs cargo. - let toolchain = stage("setup-rust", "set up Rust before anything compiles"); - let coverage = stage("generate-coverage", "build the workspace under coverage"); - let report = stage("sccache --show-stats", "report compiler-cache statistics"); - let order = [ - ("export the cache proxy", export), - ("install sccache", install), - ("start sccache", start), - ("set up the toolchain", toolchain), - ("build", coverage), - ("report the statistics", report), - ]; - for ((earlier, before), (later, after)) in order.iter().zip(order.iter().skip(1)) { - assert!( - before < after, - "`{id}` must {earlier} (step {before}) before it can {later} (step {after})" - ); - } - } -} - -#[rstest] -fn the_cache_proxy_export_is_pinned_and_names_every_variable(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let (export_at, export) = job - .first_step_with("actions/github-script") - .unwrap_or_else(|| panic!("`{id}` must export the Ubicloud cache proxy")); - assert!( - export.uses.ends_with(GITHUB_SCRIPT_SHA), - "`{id}` must pin actions/github-script to {GITHUB_SCRIPT_SHA}" - ); - let checkout_at = job - .first_step_containing("actions/checkout") - .unwrap_or_else(|| panic!("`{id}` must check out the repository")); - assert!( - checkout_at < export_at, - "`{id}` must export the proxy after checkout" - ); - let script = export.input("script"); - for variable in PROXY_VARIABLES { - assert!( - script.contains(variable), - "`{id}` must export `{variable}` for sccache's backend" - ); - } - assert!( - !script.contains("ACTIONS_RESULTS_URL"), - "`{id}` must not export ACTIONS_RESULTS_URL; it does not route \ - through Ubicloud's cache proxy" - ); - } -} - -#[rstest] -fn compiler_cache_effectiveness_is_measured_around_the_build(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let zero_at = job - .first_step_containing("sccache --zero-stats") - .unwrap_or_else(|| panic!("`{id}` must reset the compiler-cache counters")); - let (show_at, report) = job - .first_step_with("sccache --show-stats") - .unwrap_or_else(|| panic!("`{id}` must report compiler-cache statistics")); - assert!( - zero_at < show_at, - "`{id}` must reset the counters before it reports them" - ); - assert!( - report.run.contains("GITHUB_STEP_SUMMARY"), - "`{id}` must put the compiler-cache statistics in the job summary" - ); - // The summary is not readable through the REST API, so a run whose - // statistics went only there cannot be audited afterwards. - assert!( - report.run.contains("printf '%s\\n' \"$stats\""), - "`{id}` must also print the compiler-cache statistics to the log" - ); - } -} - -/// The `ubicloud-standard-8` shape is inherited here, not measured. Sampling -/// memory and disk is what turns the next shape decision into evidence, and -/// disk is the one that has killed jobs silently elsewhere in this rollout. -#[rstest] -fn both_build_jobs_sample_and_report_their_resource_use(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let start = job - .first_step_containing("sample-resources.sh") - .unwrap_or_else(|| panic!("`{id}` must start a resource sampler")); - let (report_at, report) = job - .first_step_with("least free disk") - .unwrap_or_else(|| panic!("`{id}` must report its peak resource use")); - assert!( - start < report_at, - "`{id}` must start the sampler before it reports the peaks" - ); - for measure in ["free -m", "df -m"] { - assert!( - job.steps.iter().any(|step| step.run.contains(measure)), - "`{id}` must sample `{measure}`; disk and memory are both needed" - ); - } - assert!( - report.run.contains("peak used disk"), - "`{id}` must report peak disk, not memory alone" - ); - } -} - -#[rstest] -fn coverage_runs_the_whole_suite_once_and_owns_no_cargo_cache(workflows: Vec) { - for id in BUILD_JOB_IDS { - let job = job_named(&workflows, id); - let step = step_using(job, &shared_action("generate-coverage")); - for flag in ["all-features", "all-targets", "doctests"] { - assert_input(id, step, flag, "true"); - } - assert_input(id, step, "cache-provider", "external"); - } -} - -#[rstest] -fn whitaker_is_installed_from_a_pinned_prebuilt_release(workflows: Vec) { - let job = job_named(&workflows, "build-test"); - let step = step_using(job, &shared_action("install-whitaker")); - assert_input("build-test", step, "installer-version", "0.2.7"); - assert_input("build-test", step, "cache-provider", "github"); -} - -#[rstest] -fn the_uv_cache_names_its_layers_and_keys_them_by_runner(workflows: Vec) { - let job = job_named(&workflows, "build-test"); - let cache = job - .steps - .iter() - .find(|step| step.cache_paths().iter().any(|path| path == ".uv-cache")); - let Some(step) = cache else { - panic!("`build-test` must cache the uv download layer") - }; - assert_eq!( - step.cache_paths(), - vec![".uv-cache".to_owned(), ".uv-tools".to_owned()], - "the uv cache must own both the download store and the tool store" - ); - let key = step.input("key"); - for fragment in UV_CACHE_KEY_FRAGMENTS { - assert!( - key.contains(fragment), - "the uv cache key `{key}` must vary with `{fragment}`" - ); - } -} - -#[rstest] -#[case::not_a_workflow("scratch.yml", "steps: []")] -#[case::mistyped_runner("scratch.yml", "jobs:\n a:\n runs-on: {group: [g]}\n")] -#[case::mistyped_runner_label("scratch.yml", "jobs:\n a:\n runs-on: [a, [b]]\n")] -#[case::groupless_runner_mapping("scratch.yml", "jobs:\n a:\n runs-on: {labels: [a]}\n")] -#[case::placeless_job("scratch.yml", "jobs:\n a:\n steps: []\n")] -#[case::mistyped_steps("scratch.yml", "jobs:\n a:\n runs-on: x\n steps: nope\n")] -#[case::empty_step( - "scratch.yml", - "jobs:\n a:\n runs-on: x\n steps:\n - name: n\n" -)] -#[case::mistyped_input( - "scratch.yml", - "jobs:\n a:\n runs-on: x\n steps:\n - uses: u\n with:\n k: [1]\n" -)] -// GitHub Actions runs a step either as an action or as a script, never both. -#[case::dual_mode_step( - "scratch.yml", - "jobs:\n a:\n runs-on: x\n steps:\n - uses: u\n run: echo hi\n" -)] -fn a_malformed_workflow_is_an_error_not_a_default(#[case] file: &str, #[case] text: &str) { - let outcome = parse_workflow(WorkflowSource { file, text }); - assert!( - outcome.is_err(), - "a workflow of unexpected shape must be rejected, not silently defaulted" - ); -} - -/// Every `runs-on` shape GitHub Actions accepts must parse, not just the -/// scalar one: rejecting a label list or a runner group would fail a valid -/// workflow rather than the workflow a contract is meant to catch. -#[rstest] -#[case::single_label("runs-on: ubuntu-latest\n", &["ubuntu-latest"])] -#[case::label_list("runs-on: [self-hosted, linux]\n", &["self-hosted", "linux"])] -#[case::group_only("runs-on:\n group: ubuntu-runners\n", &[])] -#[case::group_and_labels( - "runs-on:\n group: ubuntu-runners\n labels: [ubuntu-20.04-16core]\n", - &["ubuntu-20.04-16core"] -)] -fn every_valid_runs_on_shape_parses(#[case] runs_on: &str, #[case] expected: &[&str]) { - let text = format!("on: push\njobs:\n a:\n {runs_on} steps: []\n"); - let workflow = parse_workflow(WorkflowSource { - file: "scratch.yml", - text: &text, - }) - .unwrap_or_else(|err| panic!("`{runs_on}` must parse: {err}")); - let job = workflow - .jobs - .first() - .unwrap_or_else(|| panic!("`{runs_on}` must yield a job")); - assert_eq!(job.runs_on.labels(), expected); - assert!( - job.runs_on.names_a_runner(), - "`{runs_on}` names a runner and must say so" - ); -} - -#[rstest] -fn an_unreadable_workflow_directory_is_reported() { - let missing = Utf8Path::new("this/directory/does/not/exist"); - let outcome = load_workflows_in(missing); - assert!( - outcome.is_err(), - "an unreadable workflow directory must surface as an error" - ); -} diff --git a/tests/workflow_model_properties.rs b/tests/workflow_model_properties.rs index 20fc81f..fabb01b 100644 --- a/tests/workflow_model_properties.rs +++ b/tests/workflow_model_properties.rs @@ -158,6 +158,24 @@ proptest! { prop_assert_eq!(reported(&job), reported(&job_of(rotated))); } + /// An action that merely shares the `actions/cache` prefix owns nothing. + /// + /// `actions/cache-audit` is a different action. Reading it as a cache step + /// would invent a claim on whatever `path` input it happened to carry, and + /// that invented claim could report a duplicate that does not exist. + #[test] + fn a_prefixed_non_cache_action_claims_nothing( + path in prop::sample::select(PATHS.to_vec()), + filler in prop::collection::vec(any_step(), 0..4), + ) { + let mut steps = vec![ + cache_step("Cache", path), + action_step("Audit", "actions/cache-audit@sha", &[("path", path)]), + ]; + steps.extend(without_claims_on(filler, path)); + prop_assert!(!reported(&job_of(steps)).contains(path)); + } + /// Two restores sharing a key are two owners, not one half of a pair. /// /// The split-cache exception exists for one restore and one save. Applying