diff --git a/.github/workflows/release-readiness.yml b/.github/workflows/release-readiness.yml index dc27c2a..20756af 100644 --- a/.github/workflows/release-readiness.yml +++ b/.github/workflows/release-readiness.yml @@ -43,7 +43,9 @@ jobs: - uses: Swatinem/rust-cache@v2 - name: Install rust-script - run: cargo install rust-script --locked || true + # No `|| true`: a missing rust-script used to surface later as exit 127, + # indistinguishable from a verdict. Fail here, where the cause is. + run: command -v rust-script >/dev/null || cargo install rust-script --locked - name: Report shell: bash @@ -55,8 +57,17 @@ jobs: ARGS=(--markdown) [ -n "${REL:-}" ] && ARGS+=(--release "$REL") - ./scripts/release-readiness.rs "${ARGS[@]}" > /tmp/readiness.md - READY=$? + # `shell: bash` runs with -e. A bare call exiting 1 — "not ready", the + # normal state — ended this step before READY was read, so the report + # never reached the summary and the job went red every night. The + # first scheduled run (2026-09-16) did exactly that. + READY=0 + ./scripts/release-readiness.rs "${ARGS[@]}" > /tmp/readiness.md || READY=$? + if [ "$READY" -gt 1 ]; then + echo "## Release readiness: UNKNOWN — could not evaluate (exit $READY). This is not a 'not ready'; see the log." >> "$GITHUB_STEP_SUMMARY" + echo "::error::release readiness could not be evaluated (exit $READY) — see the log; this is not a 'not ready'" + exit 1 + fi { cat /tmp/readiness.md diff --git a/artifacts/swreq/SWREQ-RELAY-READY-P01.yaml b/artifacts/swreq/SWREQ-RELAY-READY-P01.yaml new file mode 100644 index 0000000..72976cd --- /dev/null +++ b/artifacts/swreq/SWREQ-RELAY-READY-P01.yaml @@ -0,0 +1,44 @@ +artifacts: + - id: SWREQ-RELAY-READY-P01 + type: sw-req + title: "READY-P01 — release readiness shall be recomputed from the tree, and shall give no verdict rather than a wrong one" + status: implemented + release: falcon-v1.139.0 + description: > + Release readiness for the next unreleased version shall be computed from + the rivet artifacts and the repository's tags, published nightly, and + reported as exactly one of: ready (every scoped artifact verified or + accepted), not ready, or COULD NOT EVALUATE. "Could not evaluate" shall + never be reported as either verdict. + + WHY IT EXISTS. Readiness was carried in an agent's head and a scratch + state file, and both drifted: the loop's pending gates still said "TAG + falcon-v1.136.0" after v1.138.0 had shipped. #423 added + `scripts/release-readiness.rs` and a nightly workflow — without a + requirement, which is itself the untracked-work gap this backfills. + + WHAT WAS WRONG WITH #423, each shown with a control against its code: + (1) The nightly job never published a report. `shell: bash` runs with -e, + so the script's exit 1 — "not ready", the normal state — ended the step + before the exit code was read. Its first scheduled run (2026-09-16) failed + with `Process completed with exit code 1` and an empty summary. + (2) An unparseable artifact file was warned about and skipped, silently + shrinking every scope. With a release's only blocker in a broken file the + tool reported "1/1 artifacts done (100%)" and exit 0 — READY. + (3) With no tags visible (a failed `git`, a shallow clone) the tag set + defaulted to empty, so the report re-targeted the oldest scope in the tree + and still issued a verdict. + (4) The next release was chosen in string order: with v1.99.0 tagged it + picked falcon-v1.100.0 over falcon-v1.99.1. + (5) Any runtime error exited 1, indistinguishable from "not ready". + + It REPORTS. It does not tag. Cutting a release stays a stop-and-ask. + + FALSIFICATION: wrong if the nightly job fails, or writes no summary, on a + tree that is merely not ready; or if the tool issues a ready/not-ready + verdict while an artifact file is unparseable or no release tags are + visible. + tags: [requirement, relay, release, readiness, verification-gate, v1.139] + links: + - type: derives-from + target: SYSREQ-FALCON-010 diff --git a/artifacts/verification/FV-RELAY-READY-001.yaml b/artifacts/verification/FV-RELAY-READY-001.yaml new file mode 100644 index 0000000..f2edc3d --- /dev/null +++ b/artifacts/verification/FV-RELAY-READY-001.yaml @@ -0,0 +1,48 @@ +artifacts: + - id: FV-RELAY-READY-001 + type: sw-verification + title: "READY-P01 — fixture repos pin all four verdict bugs; the nightly step survives `bash -e` (v1.139)" + status: implemented + release: falcon-v1.139.0 + description: > + Verifies SWREQ-RELAY-READY-P01 with fixture repositories built by the + tool's own tests, plus the workflow wiring. + + THE TESTS. `run_at(root, …)` reads only `root`, so each test builds a + throwaway git repo (tags point at a blob — no commit, no signing) with + chosen artifact files: ready → 0; not ready → 1; the release's only + blocker in an unparseable file → no verdict; no tags → no verdict; v1.99.0 + tagged with v1.99.1 and v1.100.0 scoped → targets v1.99.1. + + THE TESTS CAN FAIL. Three mutants, each reverting one fix — the + unparseable bail, the no-tags bail, and version ordering (back to map + order) — each fail exactly the one matching test (3 passed, 1 failed). + + CONTROLS AGAINST #423's CODE (recorded, 2026-09-16): the old script on the + hidden-blocker fixture printed "1/1 artifacts done (100%)" and exited 0; + on the no-tags fixture it exited 0; on the ordering fixture it targeted + falcon-v1.100.0. The workflow's Report block, extracted and run under + `bash --noprofile --norc -eo pipefail` with a stub script: new block — + exit 0 and 1 finish the step with an 11-line summary, exit 2 and 127 fail + the step with an ::error:: and an UNKNOWN summary line; old block with + exit 1 — step exit 1, no summary written, matching the real 2026-09-16 + scheduled run (job log: `Process completed with exit code 1`). + + On the real tree the refactored tool still reports falcon-v1.139.0, + 0 of 9 done, exit 1, in both text and --markdown forms. + + NOT CLAIMED: a green scheduled run on GitHub. That is observable only after + merge and is the evidence for promotion to `verified` in a separate change. + tags: [verification, relay, release, readiness, v1.139] + fields: + method: automated-test + steps: + # The four verdict bugs, against fixture repos. + - run: "rust-script --test scripts/release-readiness.rs" + # The exit code survives `shell: bash`'s -e. + - run: "grep -q '/tmp/readiness.md || READY=$?' .github/workflows/release-readiness.yml" + # A failed rust-script install fails where it happens, not as a verdict. + - run: "! grep -q 'cargo install rust-script --locked || true' .github/workflows/release-readiness.yml" + links: + - type: verifies + target: SWREQ-RELAY-READY-P01 diff --git a/scripts/release-readiness.rs b/scripts/release-readiness.rs index 650e675..ccddbe9 100755 --- a/scripts/release-readiness.rs +++ b/scripts/release-readiness.rs @@ -16,6 +16,14 @@ //! cuttable when every artifact scoped to it is `verified` (or `accepted`) and //! nothing in its scope is still open. //! +//! Exit codes — a verdict, or the honest absence of one: +//! 0 every artifact scoped to the release is done +//! 1 the release still has blocking artifacts (the normal state) +//! 2 could not evaluate: an unparseable artifact file, no release tags +//! visible, or any other error. NEVER folded into 0 or 1 — a report that +//! silently dropped a file, or measured the wrong release, is not a +//! "not ready", it is no report at all. +//! //! Usage: //! scripts/release-readiness.rs # the next unreleased version //! scripts/release-readiness.rs --release falcon-v1.139.0 @@ -60,7 +68,17 @@ fn is_done(status: &str) -> bool { matches!(status, "verified" | "accepted") } -fn main() -> Result<()> { +fn main() -> std::process::ExitCode { + match run() { + Ok(code) => std::process::ExitCode::from(code), + Err(e) => { + eprintln!("release-readiness: could not evaluate: {e:#}"); + std::process::ExitCode::from(2) + } + } +} + +fn run() -> Result { let args: Vec = std::env::args().skip(1).collect(); let markdown = args.iter().any(|a| a == "--markdown"); let want = args @@ -68,21 +86,31 @@ fn main() -> Result<()> { .position(|a| a == "--release") .and_then(|i| args.get(i + 1)) .cloned(); + run_at(std::path::Path::new("."), want, markdown) +} +/// Everything below reads `root` only, so the tests can point it at a fixture. +fn run_at(root: &std::path::Path, want: Option, markdown: bool) -> Result { let mut by_release: BTreeMap> = BTreeMap::new(); - for e in walkdir::WalkDir::new("artifacts") - .into_iter() - .filter_map(|e| e.ok()) - .filter(|e| e.path().extension().is_some_and(|x| x == "yaml")) - { + let mut unparseable: Vec = Vec::new(); + // An unreadable directory entry is the same hole as an unparseable file — + // it used to be skipped by `filter_map(|e| e.ok())`. + for e in walkdir::WalkDir::new(root.join("artifacts")) { + let e = e.context("walking artifacts/")?; + if !e.path().extension().is_some_and(|x| x == "yaml") { + continue; + } let text = std::fs::read_to_string(e.path()) .with_context(|| format!("reading {}", e.path().display()))?; // A malformed artifact file must not silently shrink the scope — that is - // the empty-scope-passes shape. Report it and keep going. + // the empty-scope-passes shape. The first version of this loop warned + // and `continue`d, which is exactly that: the file's artifacts left + // every release's scope and the verdict could still read "ready". + // Collect them all, then refuse to give a verdict. let doc: Doc = match serde_yaml::from_str(&text) { Ok(d) => d, Err(err) => { - eprintln!("::warning::unparseable artifact {}: {err}", e.path().display()); + unparseable.push(format!("{}: {err}", e.path().display())); continue; } }; @@ -93,6 +121,14 @@ fn main() -> Result<()> { } } + if !unparseable.is_empty() { + anyhow::bail!( + "{} artifact file(s) could not be parsed, so every release's scope is incomplete:\n {}", + unparseable.len(), + unparseable.join("\n ") + ); + } + // Default target: the lowest release that is NOT YET TAGGED. // // "Lowest release with something incomplete" is the obvious rule and it is @@ -100,16 +136,27 @@ fn main() -> Result<()> { // artifacts left at `implemented`. A shipped release's stale statuses are a // traceability debt, not a thing the NEXT release is waiting on. The next // release is the one whose tag does not exist yet. - let tags: std::collections::HashSet = std::process::Command::new("git") + // + // No tags is an error, not an empty set. `unwrap_or_default()` here used to + // turn a failed `git` (or a clone without tags) into "nothing has shipped", + // which silently re-targets the report at the oldest scope in the tree. + let out = std::process::Command::new("git") + .arg("-C") + .arg(root) .args(["tag", "--list", "falcon-v*"]) .output() - .map(|o| { - String::from_utf8_lossy(&o.stdout) - .lines() - .map(|l| l.trim().to_string()) - .collect() - }) - .unwrap_or_default(); + .context("running `git tag`")?; + if !out.status.success() { + anyhow::bail!("`git tag` failed: {}", String::from_utf8_lossy(&out.stderr).trim()); + } + let tags: std::collections::HashSet = String::from_utf8_lossy(&out.stdout) + .lines() + .map(|l| l.trim().to_string()) + .filter(|l| !l.is_empty()) + .collect(); + if tags.is_empty() { + anyhow::bail!("no falcon-v* tags visible — a shallow clone? (the workflow needs fetch-depth: 0)"); + } // ...and it must be AHEAD of the latest tag. "Lowest untagged" is still // wrong on its own: artifacts exist scoped to falcon-v1.113.0, which was @@ -136,9 +183,12 @@ fn main() -> Result<()> { let target = match want { Some(r) => r, + // By VERSION, not by map order: the keys sort as strings, and + // "falcon-v1.100.0" < "falcon-v1.99.1" as strings. None => by_release .keys() - .find(|r| !tags.contains(*r) && ver(r) > latest) + .filter(|r| !tags.contains(*r) && ver(r) > latest) + .min_by_key(|r| ver(r)) .cloned() .unwrap_or_else(|| "(nothing scoped beyond the latest tag)".into()), }; @@ -206,5 +256,93 @@ fn main() -> Result<()> { // Exit code is information, not a gate: 0 = ready, 1 = still blocked. A // caller that wants to fail on "not ready" can; the nightly job does not. - std::process::exit(if blocking.is_empty() && !arts.is_empty() { 0 } else { 1 }); + Ok(if blocking.is_empty() && !arts.is_empty() { 0 } else { 1 }) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + use std::process::Command; + + /// A throwaway git repo holding `tags` and one artifact file per entry of + /// `files`. Tags point at a blob, so no commit (and no signing) is needed. + fn fixture(name: &str, tags: &[&str], files: &[&str]) -> PathBuf { + let d = std::env::temp_dir().join(format!("release-readiness-{}-{name}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(d.join("artifacts")).unwrap(); + let git = |args: &[&str]| { + let o = Command::new("git").arg("-C").arg(&d).args(args).output().unwrap(); + assert!(o.status.success(), "git {args:?}: {}", String::from_utf8_lossy(&o.stderr)); + String::from_utf8(o.stdout).unwrap().trim().to_string() + }; + git(&["init", "-q"]); + let blob = { + let mut c = Command::new("git") + .arg("-C") + .arg(&d) + .args(["hash-object", "-w", "--stdin"]) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap(); + use std::io::Write; + c.stdin.take().unwrap().write_all(b"fixture").unwrap(); + String::from_utf8(c.wait_with_output().unwrap().stdout).unwrap().trim().to_string() + }; + for t in tags { + git(&["tag", t, &blob]); + } + for (i, f) in files.iter().enumerate() { + std::fs::write(d.join("artifacts").join(format!("{i}.yaml")), f).unwrap(); + } + d + } + + fn art(id: &str, status: &str, release: &str) -> String { + format!("artifacts:\n - {{id: {id}, type: sw-req, title: t, status: {status}, release: {release}}}\n") + } + + #[test] + fn verdicts_for_ready_and_not_ready() { + let r = fixture("ready", &["falcon-v1.0.0"], &[&art("A", "verified", "falcon-v1.1.0")]); + assert_eq!(run_at(&r, None, false).unwrap(), 0); + let n = fixture("notready", &["falcon-v1.0.0"], &[&art("A", "proposed", "falcon-v1.1.0")]); + assert_eq!(run_at(&n, None, false).unwrap(), 1); + } + + #[test] + fn an_unparseable_file_is_no_verdict_not_a_ready_one() { + // The release's only blocker lives in a broken file. The first version + // of this tool dropped the file with a warning and reported + // "1/1 artifacts done (100%)", exit 0. + let d = fixture( + "hidden-blocker", + &["falcon-v1.0.0"], + &[ + &art("DONE", "verified", "falcon-v1.1.0"), + "artifacts:\n - {id: BLOCKER, type: sw-req, title: t, status: proposed, release: falcon-v1.1.0}\n oops: [\n", + ], + ); + let err = run_at(&d, None, false).unwrap_err().to_string(); + assert!(err.contains("could not be parsed"), "{err}"); + } + + #[test] + fn no_visible_tags_is_no_verdict() { + // `unwrap_or_default()` used to read this as "nothing has shipped". + let d = fixture("notags", &[], &[&art("A", "verified", "falcon-v1.1.0")]); + assert!(run_at(&d, None, false).is_err()); + } + + #[test] + fn the_next_release_is_chosen_by_version_not_string_order() { + // As strings, "falcon-v1.100.0" sorts before "falcon-v1.99.1". + let d = fixture( + "ordering", + &["falcon-v1.99.0"], + &[&art("NEXT", "verified", "falcon-v1.99.1"), &art("LATER", "proposed", "falcon-v1.100.0")], + ); + assert_eq!(run_at(&d, None, false).unwrap(), 0, "must target v1.99.1, which is ready"); + } }