diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 39b4e3f8..c4568426 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -566,6 +566,24 @@ jobs: fetch-depth: 0 persist-credentials: true + # `bump_version.py` shells out to Cargo (`cargo update` for both lockfiles + # and `cargo check --locked` over the fuzz workspace), so this job needs a + # toolchain of its own. Without one it silently used whatever rustc the + # runner image preinstalled — fine until the image shipped one older than + # our `rust-version = "1.94"` MSRV, which failed the locked fuzz check and + # blocked the bump on every push to main. + - uses: dtolnay/rust-toolchain@stable + + # Restore the fuzz workspace's target dir so the locked check inside the + # bump script reuses the `fuzz-check` job's build instead of compiling the + # workspace again. Restore-only: `fuzz-check` owns writing this cache. + - name: Cache Cargo registry and target directory + uses: Swatinem/rust-cache@v2 + with: + workspaces: fuzz + shared-key: fuzz-check-cache + save-if: false + - name: Set up Python uses: actions/setup-python@v4 with: diff --git a/.github/workflows/versioning.yml b/.github/workflows/versioning.yml index 6d14ca86..d31c6461 100644 --- a/.github/workflows/versioning.yml +++ b/.github/workflows/versioning.yml @@ -15,6 +15,12 @@ jobs: fetch-depth: 0 persist-credentials: true + # `bump_version.py` shells out to Cargo, so this job needs its own + # toolchain rather than whatever rustc the runner image preinstalls (which + # can be older than our `rust-version = "1.94"` MSRV). Same fix as the + # `bump-version` job in ci.yml. + - uses: dtolnay/rust-toolchain@stable + - name: Set up Python uses: actions/setup-python@v4 with: diff --git a/Dev diary/2026-07-27-ci-bump-version-toolchain.md b/Dev diary/2026-07-27-ci-bump-version-toolchain.md new file mode 100644 index 00000000..5314e76a --- /dev/null +++ b/Dev diary/2026-07-27-ci-bump-version-toolchain.md @@ -0,0 +1,135 @@ +# 2026-07-27 — main CI red: `Bump Version` had no Rust toolchain + +## Symptom + +Every push to `main` since 2026-07-26 produced a red **CI** run, while all +seven build/test jobs went green. Three consecutive runs failed identically: + +| Run | Commit | Failing job | +|---|---|---| +| 30205164652 | `ed704e6a` (#646, Blacksmith migration) | Bump Version | +| 30208231213 | `7937a53b` (#649) | Bump Version | +| 30233263720 | `f903e8e0` (#650) | Bump Version | +| 30237670286 | `48c46423` (#652) | Bump Version | + +The job log: + +```text +error: rustc 1.92.0 is not supported by the following packages: + sqlx@0.9.0 requires rustc 1.94.0 + ... + wfl@26.7.53 requires rustc 1.94 +Error: locked fuzz check failed after bump; refusing to stage a broken fuzz/Cargo.lock +``` + +## Root cause + +`bump-version` in `.github/workflows/ci.yml` runs +`python scripts/bump_version.py --update-all`. That script shells out to Cargo +three times — `cargo update --package wfl` for the root lock, the same for +`fuzz/Cargo.lock`, and finally `cargo check --locked --manifest-path +fuzz/Cargo.toml` as a guard so a bump can never stage a fuzz lock that the +`fuzz-check` gate would then reject. + +The job never installed a toolchain. Every *other* Cargo-running job in the repo +uses `dtolnay/rust-toolchain@stable`; this one silently inherited whatever +`rustc` the runner image happened to preinstall. That worked on GitHub-hosted +images and stopped working the moment CI moved to Blacksmith runners (#646), +whose `ubuntu-2404` image ships rustc 1.92.0 — below our +`rust-version = "1.94"` (raised by the `sqlx` 0.9 dependency). The locked fuzz +check failed, the script refused to stage, and the job exited 1. + +Two things made this hard to see at a glance: + +- The Cargo dependency is **invisible in the workflow file** — it hides behind a + Python script, so a reader scanning for `cargo` in `ci.yml` finds nothing in + this job. +- The failure is a *consequence* of the migration PR but appears in a job that + PR never touched beyond the `runs-on:` label. + +Because the bump never landed, `main` has been stuck at version **26.7.52** and +no `v*` tags were pushed for those four commits. + +## Fix + +Give the job the toolchain it always needed, before the bump step: + +- `.github/workflows/ci.yml` — added `dtolnay/rust-toolchain@stable`, plus a + restore-only `Swatinem/rust-cache` sharing `fuzz-check`'s `fuzz-check-cache` + key so the locked fuzz check reuses that job's build instead of compiling the + workspace a second time on every push to `main`. +- `.github/workflows/versioning.yml` — the manual-dispatch `bump-version` job + had the identical latent defect (it would have failed the same way the next + time anyone triggered it). Same one-line fix. + +Deliberately *not* changed: the `cargo check --locked` guard inside +`bump_version.py`. It is doing exactly its job — it caught a broken environment +and refused to commit a lockfile it could not verify. Weakening it to "skip when +cargo is unhappy" would trade a loud failure for a silently stale +`fuzz/Cargo.lock`. + +## Testing (Logbie Testing Policy) + +- **Risk class:** R1 — build/release tooling, no runtime behavior change. + +- **Acceptance criteria → tests** (all in `tests/workflow_rust_toolchain_test.rs`): + + | Acceptance criterion | Test | + |---|---| + | Every workflow job that runs Cargo — directly or via a `scripts/*.py` indirection — installs/selects a toolchain *before* the first Cargo use | `cargo_jobs_install_a_rust_toolchain_first` | + | The Cargo-invoking script inventory (`CARGO_INVOKING_SCRIPTS`) cannot silently go stale as scripts change | `cargo_invoking_scripts_list_is_complete` | + | The scanner ignores comment-only `cargo` mentions yet still finds real jobs | `scanner_ignores_comments_and_finds_jobs` | + | Only genuine toolchain setup counts; `echo rust-toolchain` and `rustup target/component add` do not | `toolchain_markers_reject_incidental_mentions` | + | Cargo-argv detection survives whitespace, quote-style, and multi-line reformatting of `subprocess.run([...])` | `script_cargo_detection_tolerates_whitespace_and_argv_forms` | + | A toolchain installed *after* the first Cargo step is treated as out of order, not as satisfying the rule | `toolchain_after_cargo_is_out_of_order` | + +- **Red → Green:** `tests/workflow_rust_toolchain_test.rs` was committed test-only + in `2ab115d` (an ancestor of the fix commit) and failed there for the intended + reason — the two toolchain-less jobs — before any `ci.yml`/`versioning.yml` + edit existed: + + ```text + no toolchain step: ["ci.yml:bump-version", "versioning.yml:bump-version"] + ``` + + It passes on the fix commit. The `toolchain_markers_reject_incidental_mentions` + and `script_cargo_detection_...` guards were added after review to tighten the + scanner so it matches real setup steps and Cargo argv forms rather than loose + substrings. + + The tightened scanner was then re-checked against the original defect: with the + `dtolnay/rust-toolchain@stable` step temporarily removed from `ci.yml`, + `cargo_jobs_install_a_rust_toolchain_first` fails again with + `no toolchain step: ["ci.yml:bump-version"]`. The hardening did not cost the + guard its bite. + +- **Validation evidence** (rustc 1.94.1, satisfies `rust-version = "1.94"`): + + ```text + $ cargo fmt --all -- --check + # clean, no diff + + $ cargo clippy --all-targets --all-features -- -D warnings + Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 41s # no warnings + + $ cargo test --test workflow_rust_toolchain_test + test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out + ``` + + Full workspace suite: CI run 30240210815 on this branch — `Build, Test, Clippy` + green, including its `Run Tests` step, alongside the integration, database, + fuzz-compile, and WFL-program jobs on both Linux and Windows. (A local + `cargo test --all` in the authoring container exhausted its disk allowance + mid-link — a `Bus error` from the ~30 GB `target/` tree described in + `CLAUDE.md`, not a test failure; CI runners carry the `Free disk space` step + that the container lacks.) + +- **Real boundary:** `cargo check --locked --manifest-path fuzz/Cargo.toml` — + the exact command that failed in CI — was run locally on an MSRV-satisfying + toolchain (rustc 1.94.1) and passes, confirming an adequate toolchain is the + whole of the fix. +- **Residual risk:** the guard is a line scanner, not a YAML parse (the repo has + no YAML dependency), so it assumes job ids are the only two-space-indented + keys under `jobs:` — true for all current workflows and asserted by the + scanner self-test. It also cannot see Cargo invoked from a shell script or a + composite action; only `scripts/*.py` indirection is resolved. diff --git a/tests/workflow_rust_toolchain_test.rs b/tests/workflow_rust_toolchain_test.rs new file mode 100644 index 00000000..d005af16 --- /dev/null +++ b/tests/workflow_rust_toolchain_test.rs @@ -0,0 +1,335 @@ +//! Guard: every GitHub Actions job that runs Cargo must install a Rust toolchain +//! *before* the first Cargo invocation. +//! +//! Background (the defect this reproduces): the `bump-version` job in `ci.yml` +//! ran `python scripts/bump_version.py --update-all` — which shells out to +//! `cargo update` and `cargo check --locked --manifest-path fuzz/Cargo.toml` — +//! without ever installing a toolchain. It silently depended on whatever `rustc` +//! the runner image happened to preinstall. When CI moved to Blacksmith runners +//! that image shipped rustc 1.92.0, below this crate's `rust-version = "1.94"` +//! (raised by `sqlx` 0.9), so the locked fuzz check failed and every push to +//! `main` went red. +//! +//! The Cargo dependency was invisible to a reader because it hid behind a Python +//! script, so this guard resolves that indirection explicitly via +//! `CARGO_INVOKING_SCRIPTS` — and `cargo_invoking_scripts_list_is_complete` +//! keeps that list honest as scripts change. + +use std::collections::BTreeSet; +use std::fs; +use std::path::PathBuf; + +/// Scripts that shell out to Cargo. A job that runs one of these needs a +/// toolchain just as much as a job that types `cargo` directly. +const CARGO_INVOKING_SCRIPTS: &[&str] = &["bump_version.py"]; + +/// Markers for a step that *actually* installs or selects a Rust toolchain. +/// +/// Deliberately specific action paths and rustup subcommands rather than the +/// loose substrings `rust-toolchain` / `rustup `: those would be satisfied by +/// `echo rust-toolchain` or `rustup target add …` / `rustup component add …`, +/// none of which pin a compiler compatible with this crate's MSRV. Matching the +/// real setup steps keeps the guard from passing on cosmetic mentions. +const TOOLCHAIN_MARKERS: &[&str] = &[ + "dtolnay/rust-toolchain", // the action this repo uses + "actions-rust-lang/setup-rust-toolchain", // common alternative + "actions-rs/toolchain", // legacy action + "rustup toolchain install", // explicit install + "rustup default", // select the active toolchain + "rustup override set", // per-directory pin +]; + +fn repo_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) +} + +fn workflow_files() -> Vec { + let dir = repo_root().join(".github").join("workflows"); + let mut files: Vec = fs::read_dir(&dir) + .unwrap_or_else(|e| panic!("cannot read {}: {e}", dir.display())) + .map(|entry| entry.expect("cannot read workflow dir entry").path()) + .filter(|path| { + path.extension() + .is_some_and(|ext| ext == "yml" || ext == "yaml") + }) + .collect(); + files.sort(); + assert!( + !files.is_empty(), + "no workflow files found under {}", + dir.display() + ); + files +} + +/// Split a workflow into `(job_id, job_body)` pairs. +/// +/// Deliberately a line scanner rather than a YAML parse: the repo has no YAML +/// dependency, and job ids are the only two-space-indented keys inside `jobs:`. +/// Full-line comments are dropped so prose mentioning `cargo` cannot be mistaken +/// for a real invocation. +fn jobs_of(source: &str) -> Vec<(String, String)> { + let mut jobs: Vec<(String, Vec<&str>)> = Vec::new(); + let mut in_jobs = false; + + for line in source.lines() { + if !in_jobs { + in_jobs = line.trim_end() == "jobs:"; + continue; + } + // A new top-level key ends the `jobs:` mapping. + if !line.is_empty() && !line.starts_with(char::is_whitespace) && !line.starts_with('#') { + in_jobs = false; + continue; + } + if line.trim_start().starts_with('#') { + continue; + } + if let Some(job_id) = job_header(line) { + jobs.push((job_id, Vec::new())); + continue; + } + if let Some((_, body)) = jobs.last_mut() { + body.push(line); + } + } + + jobs.into_iter() + .map(|(id, body)| (id, body.join("\n"))) + .collect() +} + +/// ` job-id:` — exactly two spaces of indent, nothing after the colon. +fn job_header(line: &str) -> Option { + let rest = line.strip_prefix(" ")?; + if rest.starts_with(' ') { + return None; + } + let id = rest.strip_suffix(':')?; + if id.is_empty() + || !id + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + { + return None; + } + Some(id.to_string()) +} + +fn first_match(haystack: &str, needles: &[&str]) -> Option { + needles.iter().filter_map(|n| haystack.find(n)).min() +} + +fn first_cargo_use(body: &str) -> Option { + let mut needles: Vec<&str> = vec!["cargo "]; + needles.extend_from_slice(CARGO_INVOKING_SCRIPTS); + first_match(body, &needles) +} + +/// Does this Python source shell out to Cargo through a subprocess argv literal +/// whose *program* (first element) is `cargo`? +/// +/// Collapsing all whitespace before matching makes the check tolerant of argv +/// formatting: `subprocess.run([ "cargo", … ])`, a `['cargo', …]` single-quoted +/// list, and an argv split across several lines all normalize to the same +/// `["cargo"` / `['cargo'` token. A bare `"cargo"` in prose, a comment, or a +/// non-leading argv position (e.g. `["python", "cargo_helper.py"]`) does not +/// match, so this keeps the "program is cargo" meaning of the original check. +fn script_invokes_cargo(source: &str) -> bool { + let collapsed: String = source.split_whitespace().collect(); + collapsed.contains("[\"cargo\"") || collapsed.contains("['cargo'") +} + +#[test] +fn cargo_jobs_install_a_rust_toolchain_first() { + let mut missing = Vec::new(); + let mut out_of_order = Vec::new(); + + for path in workflow_files() { + let source = fs::read_to_string(&path).expect("cannot read workflow file"); + let name = path.file_name().unwrap().to_string_lossy().to_string(); + + for (job, body) in jobs_of(&source) { + let Some(cargo_at) = first_cargo_use(&body) else { + continue; + }; + match first_match(&body, TOOLCHAIN_MARKERS) { + None => missing.push(format!("{name}:{job}")), + Some(toolchain_at) if toolchain_at > cargo_at => { + out_of_order.push(format!("{name}:{job}")) + } + Some(_) => {} + } + } + } + + assert!( + missing.is_empty() && out_of_order.is_empty(), + "GitHub Actions jobs run Cargo without a pinned Rust toolchain, so they \ + inherit whatever rustc the runner image ships — which can be older than \ + this crate's MSRV (rust-version = \"1.94\").\n\ + \n no toolchain step: {missing:?}\ + \n toolchain installed after the first Cargo use: {out_of_order:?}\n\ + \nAdd `- uses: dtolnay/rust-toolchain@stable` before the Cargo step.\n\ + Jobs invoking Cargo indirectly through {CARGO_INVOKING_SCRIPTS:?} count too.", + ); +} + +/// The indirection list is load-bearing: if a new script starts shelling out to +/// Cargo and nobody adds it here, the guard above silently stops guarding. +#[test] +fn cargo_invoking_scripts_list_is_complete() { + let scripts_dir = repo_root().join("scripts"); + let declared: BTreeSet<&str> = CARGO_INVOKING_SCRIPTS.iter().copied().collect(); + let mut undeclared = Vec::new(); + + for entry in fs::read_dir(&scripts_dir).expect("cannot read scripts dir") { + let path = entry.expect("cannot read scripts dir entry").path(); + if path.extension().is_none_or(|ext| ext != "py") { + continue; + } + let source = fs::read_to_string(&path).expect("cannot read script"); + if !script_invokes_cargo(&source) { + continue; + } + let name = path.file_name().unwrap().to_string_lossy().to_string(); + if !declared.contains(name.as_str()) { + undeclared.push(name); + } + } + + assert!( + undeclared.is_empty(), + "these scripts invoke cargo but are missing from CARGO_INVOKING_SCRIPTS, \ + so workflow jobs running them would not be checked for a toolchain: {undeclared:?}", + ); +} + +/// Guards the scanner itself: a body that mentions cargo only in a comment must +/// not be treated as a Cargo user, and a real job must be found. +#[test] +fn scanner_ignores_comments_and_finds_jobs() { + let workflow = "\ +name: Example +jobs: + commented: + runs-on: ubuntu-latest + steps: + # this job used to run cargo build + - run: echo hi + real: + runs-on: ubuntu-latest + steps: + - run: cargo build +"; + let jobs = jobs_of(workflow); + let ids: Vec<&str> = jobs.iter().map(|(id, _)| id.as_str()).collect(); + assert_eq!(ids, ["commented", "real"]); + + assert_eq!( + first_cargo_use(&jobs[0].1), + None, + "comment counted as cargo" + ); + assert!( + first_cargo_use(&jobs[1].1).is_some(), + "missed a real cargo run" + ); +} + +/// Ordering is half the contract — installing the toolchain *after* the first +/// Cargo step fails exactly like not installing it at all, but the main guard +/// only reports that case when a job trips it, so pin the comparison here. +#[test] +fn toolchain_after_cargo_is_out_of_order() { + let workflow = "\ +name: Example +jobs: + late: + runs-on: ubuntu-latest + steps: + - run: cargo build + - uses: dtolnay/rust-toolchain@stable + early: + runs-on: ubuntu-latest + steps: + - uses: dtolnay/rust-toolchain@stable + - run: cargo build +"; + let jobs = jobs_of(workflow); + + let late = &jobs[0].1; + assert!( + first_match(late, TOOLCHAIN_MARKERS).expect("no toolchain marker") + > first_cargo_use(late).expect("no cargo use"), + "late fixture should place the toolchain after the first Cargo use" + ); + + let early = &jobs[1].1; + assert!( + first_match(early, TOOLCHAIN_MARKERS).expect("no toolchain marker") + < first_cargo_use(early).expect("no cargo use"), + "early fixture should place the toolchain before the first Cargo use" + ); +} + +/// Pins toolchain-marker semantics: genuine setup/selection steps count, while +/// cosmetic mentions (`echo`) and unrelated `rustup` subcommands (adding a +/// target or component) must not, so a job that never pins a compiler can't +/// sneak past the guard. +#[test] +fn toolchain_markers_reject_incidental_mentions() { + for real in [ + "- uses: dtolnay/rust-toolchain@stable", + "- uses: actions-rust-lang/setup-rust-toolchain@v1", + "- uses: actions-rs/toolchain@v1", + "run: rustup toolchain install 1.94.0", + "run: rustup default 1.94.0", + "run: rustup override set 1.94.0", + ] { + assert!( + first_match(real, TOOLCHAIN_MARKERS).is_some(), + "real toolchain setup not recognized: {real:?}" + ); + } + + for incidental in [ + "run: echo rust-toolchain", + "run: rustup target add x86_64-unknown-linux-musl", + "run: rustup component add clippy rustfmt", + ] { + assert!( + first_match(incidental, TOOLCHAIN_MARKERS).is_none(), + "incidental mention wrongly counted as toolchain setup: {incidental:?}" + ); + } +} + +/// Pins the Cargo-argv detector across whitespace and quoting variants so a +/// reformatting of a script's `subprocess.run(...)` call can't silently drop it +/// from the completeness check. +#[test] +fn script_cargo_detection_tolerates_whitespace_and_argv_forms() { + for invokes in [ + r#"subprocess.run(["cargo", "update", "--package", "wfl"])"#, + r#"subprocess.run(['cargo', 'update'])"#, + r#"subprocess.run([ "cargo", "check" ])"#, + "subprocess.run([\n \"cargo\",\n \"build\",\n])", + ] { + assert!( + script_invokes_cargo(invokes), + "argv invoking cargo not detected: {invokes:?}" + ); + } + + for benign in [ + "# this helper wraps cargo update\nprint(\"cargo\")", + r#"subprocess.run(["python", "cargo_helper.py"])"#, + r#"LEADS = ("wfl ", "$", "cargo", "npm")"#, + ] { + assert!( + !script_invokes_cargo(benign), + "non-invocation wrongly flagged as a cargo argv: {benign:?}" + ); + } +}