fix(release): the nightly readiness report never published — and could call a release ready it had not read

.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

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

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

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,29 +68,49 @@ 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<u8> {
     let args: Vec<String> = std::env::args().skip(1).collect();
     let markdown = args.iter().any(|a| a == "--markdown");
     let want = args
         .iter()
         .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<String>, markdown: bool) -> Result<u8> {
     let mut by_release: BTreeMap<String, Vec<Artifact>> = 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<String> = 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,23 +121,42 @@ 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
     // wrong: it selects falcon-v0.1.0, which shipped long ago and still carries
     // 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<String> = 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> = 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");
+    }
 }