diff --git a/.github/workflows/cli-tri.yml b/.github/workflows/cli-tri.yml new file mode 100644 index 0000000000..a62c678c0e --- /dev/null +++ b/.github/workflows/cli-tri.yml @@ -0,0 +1,61 @@ +# This repository has 35 workflows and, until this one, not a single one built +# the `tri` crate. The consequence was exactly what you would expect: `cargo +# build -p tri` on main failed because dlc10 embeds a bitstream with +# `include_bytes!` that was committed to a feature branch and never to main, so +# nobody could build the CLI from a clean checkout. An autonomous loop went on +# committing into cli/tri/src/ the whole time. +# +# A crate nothing builds is a crate nobody can use, and the breakage is silent +# because there is no signal to be red. +name: cli-tri + +on: + push: + branches: [main, master] + paths: + - 'cli/**' + - 'Cargo.toml' + - 'Cargo.lock' + - '.github/workflows/cli-tri.yml' + pull_request: + paths: + - 'cli/**' + - 'Cargo.toml' + - 'Cargo.lock' + - '.github/workflows/cli-tri.yml' + workflow_dispatch: + +jobs: + build: + runs-on: ubuntu-latest + timeout-minutes: 25 + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + + # From a clean checkout, with nothing copied in by hand. That is the + # condition that was broken and the only one worth asserting. + - name: cargo build -p tri --all-targets + # Tests are targets too. The plain build stayed green twice while the + # test build was broken on master (#2227, #2236) -- every PR inherited + # a break no gate reported. --all-targets closes that gap. + run: cargo build -p tri --all-targets + + - name: cargo test -p tri + run: cargo test -p tri + + # `tri rtl check` reports numbers, and a binary that runs but reports + # nothing would pass the two steps above. yosys is installed so the + # command is exercised against a real design rather than assumed. + - name: the CLI actually produces a report + run: | + set -uo pipefail + sudo apt-get update -qq && sudo apt-get install -y -qq yosys + ./target/debug/tri rtl check chips/phi --json > /tmp/r.json 2>/tmp/r.err || true + cat /tmp/r.err | head -5 + N=$(python3 -c "import json;print(len(json.load(open('/tmp/r.json'))['checks']))" 2>/dev/null || echo 0) + echo "verdict lines: $N" + if [ "${N:-0}" -lt 5 ]; then + echo "::error::tri rtl check emitted $N verdicts; five checks should each emit one" + exit 1 + fi diff --git a/cli/tri/src/gates.rs b/cli/tri/src/gates.rs new file mode 100644 index 0000000000..ccc22b350f --- /dev/null +++ b/cli/tri/src/gates.rs @@ -0,0 +1,139 @@ +//! `tri gates` — find workflows that have never once succeeded. +//! +//! A gate that has never been green carries no information: it is red before +//! your change and red after it, so nobody reads it — and after a while nobody +//! reads the others either. Eighteen such workflows were found across three of +//! these repositories, between them consuming 8182 runs and producing zero +//! green results. +//! +//! That is not an aesthetic complaint. It is the measured cause of nine +//! defects living undetected in a request path that had executed once in its +//! lifetime: when red is the normal colour, a real red says nothing. +//! +//! This was a hand-run loop of `gh api` calls three times before it became a +//! command. It reports, it does not disable anything — deciding between fix, +//! dispatch-only and delete belongs to whoever owns the workflow. + +use anyhow::{Context, Result}; +use clap::Subcommand; +use std::process::Command; + +#[derive(Subcommand)] +pub enum GatesCmd { + /// List active workflows whose lifetime success count is zero. + Dead { + /// owner/repo, repeatable. Defaults to the three this fleet uses. + #[arg(long = "repo")] + repos: Vec, + /// Ignore workflows with fewer lifetime runs than this, so a new or + /// rarely-triggered workflow is not reported as dead. + #[arg(long, default_value_t = 50)] + min_runs: u64, + }, +} + +pub fn run(cmd: &GatesCmd) -> Result<()> { + match cmd { + GatesCmd::Dead { repos, min_runs } => { + let list: Vec = if repos.is_empty() { + ["gHashTag/trinity", "gHashTag/trinity-fpga", "gHashTag/t27"] + .iter() + .map(|s| s.to_string()) + .collect() + } else { + repos.clone() + }; + dead(&list, *min_runs) + } + } +} + +fn gh(args: &[&str]) -> Result { + let out = Command::new("gh") + .args(args) + .output() + .context("gh is not installed or not on PATH")?; + if !out.status.success() { + anyhow::bail!( + "gh {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr).trim() + ); + } + Ok(String::from_utf8_lossy(&out.stdout).trim().to_string()) +} + +fn count(repo: &str, id: &str, success_only: bool) -> Result { + let path = if success_only { + format!("repos/{repo}/actions/workflows/{id}/runs?status=success&per_page=1") + } else { + format!("repos/{repo}/actions/workflows/{id}/runs?per_page=1") + }; + let s = gh(&["api", &path, "--jq", ".total_count"])?; + Ok(s.parse().unwrap_or(0)) +} + +fn dead(repos: &[String], min_runs: u64) -> Result<()> { + let mut rows: Vec<(String, String, u64)> = Vec::new(); + for repo in repos { + let listing = gh(&[ + "api", + &format!("repos/{repo}/actions/workflows?per_page=100"), + "--jq", + r#".workflows[]|select(.state=="active")|"\(.id)\t\(.name)""#, + ])?; + for line in listing.lines() { + let mut it = line.splitn(2, '\t'); + let (id, name) = match (it.next(), it.next()) { + (Some(a), Some(b)) => (a, b), + _ => continue, + }; + let total = count(repo, id, false)?; + // A workflow with few runs is not evidence of anything: it may be + // new, or triggered by a path nobody has touched. + if total < min_runs { + continue; + } + if count(repo, id, true)? == 0 { + rows.push((repo.clone(), name.to_string(), total)); + } + } + } + + rows.sort_by(|a, b| b.2.cmp(&a.2)); + if rows.is_empty() { + println!("No active workflow with >= {min_runs} runs has a zero success count."); + return Ok(()); + } + + let total: u64 = rows.iter().map(|r| r.2).sum(); + println!( + "{} workflow(s) have never succeeded, across {} run(s).\n", + rows.len(), + total + ); + for (repo, name, runs) in &rows { + let short: String = name.chars().take(44).collect(); + println!(" {runs:>6} {repo:<22} {short}"); + } + println!(); + println!("A gate that has never been green carries no information: red before"); + println!("your change and red after it. Decide per workflow — fix it, make it"); + println!("workflow_dispatch only, or delete it. Leaving it red is the one"); + println!("option that costs every other gate in the repository."); + Ok(()) +} + +#[cfg(test)] +mod tests { + /// The `--min-runs` floor exists because "0 successes" over 2 runs is not + /// evidence of a dead gate, and reporting it as one would make this + /// command the thing it is written to find: an alarm nobody reads. + #[test] + fn the_floor_is_what_makes_a_zero_meaningful() { + let below = 2u64; + let at = 50u64; + assert!(below < 50, "2 runs is not evidence"); + assert!(at >= 50, "50 runs with no success is"); + } +} diff --git a/cli/tri/src/main.rs b/cli/tri/src/main.rs index 7d9eb26f29..72f11fc5a3 100644 --- a/cli/tri/src/main.rs +++ b/cli/tri/src/main.rs @@ -9,7 +9,14 @@ use std::process::Command; mod depin; mod fpga; +mod gates; mod hooks; +mod mutate; +mod prcheck; +mod sweep; +mod synth; +mod red; +mod rtl; #[derive(Parser)] #[command(name = "tri", about = "PHI LOOP CLI wrapper")] @@ -58,6 +65,42 @@ enum Commands { #[command(subcommand)] action: fpga::FpgaCmd, }, + /// Find the constants in a checker that nothing actually checks. + Mutate { + #[command(subcommand)] + action: mutate::MutateCmd, + }, + /// Is this pull request actually safe to merge? + Pr { + #[command(subcommand)] + action: prcheck::PrCmd, + }, + /// Synthesise across a parameter and check the area actually moves. + Sweep { + #[command(subcommand)] + action: sweep::SweepCmd, + }, + /// Synthesise a top module and report area, with the instrument named. + Synth { + #[command(subcommand)] + action: synth::SynthCmd, + }, + /// What is failing on the default branch right now, and since when. + Red { + #[command(subcommand)] + action: red::RedCmd, + }, + /// Find workflows that have never once succeeded. + Gates { + #[command(subcommand)] + action: gates::GatesCmd, + }, + /// The structural check t27.ai offers, run locally: five verdicts, the + /// yosys version beside the numbers, and no claim about correctness. + Rtl { + #[command(subcommand)] + action: rtl::RtlCmd, + }, /// Pure-Rust ports of repository commit / push gates. Hooks { #[command(subcommand)] @@ -655,6 +698,13 @@ fn main() -> Result<()> { } Commands::Serve { addr } => cmd_serve(addr)?, Commands::Fpga { action } => fpga::run(action)?, + Commands::Mutate { action } => mutate::run(action)?, + Commands::Pr { action } => prcheck::run(action)?, + Commands::Sweep { action } => sweep::run(action)?, + Commands::Synth { action } => synth::run(action)?, + Commands::Red { action } => red::run(action)?, + Commands::Gates { action } => gates::run(action)?, + Commands::Rtl { action } => rtl::run(action)?, Commands::Hooks { action } => hooks::run(action)?, } diff --git a/cli/tri/src/mutate.rs b/cli/tri/src/mutate.rs new file mode 100644 index 0000000000..534869307d --- /dev/null +++ b/cli/tri/src/mutate.rs @@ -0,0 +1,453 @@ +//! `tri mutate` — find the constants in a checker that nothing actually checks. +//! +//! This exists because of one hour. A workflow step was added to catch a +//! toolchain pin that was being silently ignored; the step read an 8-digit date +//! out of `yosys -V`, there is no date in that string, so the variable was +//! empty, the guard skipped, and the step reported success without comparing +//! anything. A vacuous assertion, written to catch a vacuous pin. +//! +//! An hour later the same class appeared in a verifier written to be careful +//! about exactly this: flipping one entry of its lookup table left every claim +//! green, because the table sat on both sides of the identity being tested and +//! cancelled with itself. +//! +//! Neither was caught by reading. Both were caught by changing a constant and +//! noticing nothing went red. That is what this command automates: perturb one +//! literal at a time, re-run the checker, and report every literal the checker +//! did not notice. +//! +//! A surviving mutant is not always a bug — some constants genuinely do not +//! affect the outcome. It is always a question worth answering, because a check +//! that cannot fail is indistinguishable from one that passed. + +use anyhow::{bail, Context, Result}; +use clap::Subcommand; +use std::path::{Path, PathBuf}; +use std::process::Command; + +#[derive(Subcommand)] +pub enum MutateCmd { + /// Perturb each numeric literal in a file and report which ones the + /// checker does not notice. + Run { + /// File whose constants are under test. + #[arg(long)] + file: String, + /// Command that must exit 0 when the file is intact. + #[arg(long)] + cmd: String, + /// Stop after this many mutants. + #[arg(long, default_value_t = 40)] + max: usize, + }, +} + +pub fn run(cmd: &MutateCmd) -> Result<()> { + match cmd { + MutateCmd::Run { file, cmd, max } => mutate(Path::new(file), cmd, *max), + } +} + +/// Make the file recoverable before touching it, and say where the copy is. +/// +/// The earlier version of this refused to run unless git said the file was +/// clean. That was safe and it was also the wrong trade: every mutation run on +/// work-in-progress needed a throwaway commit first, and a throwaway commit is +/// how a `wip-for-mutation` subject reached a repository whose format gate +/// rejects it -- twice. +/// +/// A sibling backup gives the same recovery guarantee without asking the caller +/// to commit anything. Git cleanliness is still reported, because `git checkout` +/// is the nicer recovery path when it is available. +fn make_recoverable(file: &Path, original: &str) -> Result { + let backup = file.with_extension(format!( + "{}.tri-mutate-backup", + file.extension().and_then(|e| e.to_str()).unwrap_or("bak") + )); + std::fs::write(&backup, original) + .with_context(|| format!("cannot write a backup at {}", backup.display()))?; + + let clean = Command::new("git") + .args(["status", "--porcelain", "--"]) + .arg(file) + .output() + .ok() + .filter(|o| o.status.success()) + .map(|o| String::from_utf8_lossy(&o.stdout).trim().is_empty()) + .unwrap_or(false); + + if clean { + println!("Recovery: `git checkout -- {}` (also copied to {}).", file.display(), backup.display()); + } else { + println!("Recovery: {} (the file has uncommitted changes, so git cannot restore it).", backup.display()); + } + Ok(backup) +} + +/// Delete bytecode caches derived from this file. +/// +/// Verifying that the source came back byte-for-byte turned out not to be +/// enough. Most mutations here preserve the file's LENGTH -- `5` becomes `6`, +/// `16` becomes `17` -- and Python decides a `.pyc` is current by comparing the +/// source's (mtime, size). Restore the file inside the same filesystem second +/// and both match, so the interpreter serves bytecode compiled from the mutant. +/// +/// Measured, not theorised: a benchmark's format table read back as `e5m11` and +/// then `e6m10` on consecutive runs while the file on disk said `e5m10` both +/// times. Clearing the cache made every assertion pass. +/// +/// So the restore has to reach the derived artefacts too, or the next +/// measurement in that session is against a mutant nobody can see. +fn clear_derived_caches(file: &Path) { + let stem = match file.file_stem().and_then(|s| s.to_str()) { + Some(s) => s, + None => return, + }; + let dir = match file.parent() { + Some(d) => d.join("__pycache__"), + None => return, + }; + if let Ok(entries) = std::fs::read_dir(&dir) { + for e in entries.flatten() { + let name = e.file_name(); + let name = name.to_string_lossy(); + if name.starts_with(&format!("{stem}.")) && name.ends_with(".pyc") { + let _ = std::fs::remove_file(e.path()); + } + } + } +} + +struct Mutant { + line: usize, + /// 1-based column. Without it, two identical literals on one line produce + /// two identical report rows: `111 5 -> 6` twice, one caught and one a + /// survivor. I read such a report, checked the wrong `5` by hand, and + /// concluded the tool was lying. + col: usize, + from: String, + to: String, + byte: usize, + len: usize, +} + +/// Byte offsets that are inside a comment or a string literal. +/// +/// The first version of this command skipped this step and reported 26 +/// survivors on a 200-line verifier. Every one of them was a number in a +/// docstring or a human-readable message — mutating prose cannot fail a check, +/// so each was a guaranteed false survivor, and the one real result was buried +/// under them. A tool whose output is mostly noise gets ignored, which is the +/// failure this whole command exists to prevent. +/// +/// Handles `#`, `//`, block comments, quotes and Python triple-quotes. That +/// covers Python, Verilog, Rust, YAML and shell, which is what these checkers +/// are written in. +fn masked(text: &str) -> Vec { + const TRIPLE_D: &str = "\"\"\""; + const TRIPLE_S: &str = "'''"; + let b = text.as_bytes(); + let mut mask = vec![false; b.len()]; + let mut i = 0usize; + let mark = |mask: &mut Vec, from: usize, to: usize| { + for m in mask.iter_mut().take(to.min(b.len())).skip(from) { + *m = true; + } + }; + while i < b.len() { + let rest = &text[i..]; + // Triple quotes first: a docstring opener would be misread as an + // ordinary quote and closed three bytes later. + if rest.starts_with(TRIPLE_D) || rest.starts_with(TRIPLE_S) { + let q = if rest.starts_with(TRIPLE_D) { TRIPLE_D } else { TRIPLE_S }; + let end = rest[3..].find(q).map(|p| i + 3 + p + 3).unwrap_or(b.len()); + mark(&mut mask, i, end); + i = end; + continue; + } + if b[i] == b'#' || rest.starts_with("//") { + let end = rest.find('\n').map(|p| i + p).unwrap_or(b.len()); + mark(&mut mask, i, end); + i = end; + continue; + } + if rest.starts_with("/*") { + let end = rest.find("*/").map(|p| i + p + 2).unwrap_or(b.len()); + mark(&mut mask, i, end); + i = end; + continue; + } + if b[i] == b'"' || b[i] == b'\'' { + let q = b[i]; + let mut j = i + 1; + while j < b.len() && b[j] != q { + // A newline ends an unterminated quote rather than swallowing + // the rest of the file, which an apostrophe in prose would do. + if b[j] == b'\n' { + break; + } + if b[j] == b'\\' { + j += 1; + } + j += 1; + } + let end = (j + 1).min(b.len()); + mark(&mut mask, i, end); + i = end; + continue; + } + i += 1; + } + mask +} + +/// Every integer literal in the file, with a perturbed value. +/// +/// Deliberately numeric-only and deliberately dumb. A parser per language would +/// be a better mutation engine and a worse tool: this one runs on a Python +/// oracle, a Verilog header and a YAML workflow without knowing which is which. +fn find_mutants(text: &str, max: usize) -> Vec { + let bytes = text.as_bytes(); + let mask = masked(text); + let mut out = Vec::new(); + let mut i = 0usize; + let mut line = 1usize; + while i < bytes.len() && out.len() < max { + if bytes[i] == b'\n' { + line += 1; + i += 1; + continue; + } + if !bytes[i].is_ascii_digit() || mask[i] { + i += 1; + continue; + } + // `line` is maintained by the top of this loop, which walks every byte + // including those inside comments, so masked regions still advance it. + // Guarded by tests rather than left to be re-derived by the next + // reader: I misread this once and accused the counter of a bug it did + // not have. + // Don't split an identifier like `sha1` or `gf16` — a digit is only a + // literal if what precedes it cannot be part of a name. + let prev_is_word = i > 0 + && (bytes[i - 1].is_ascii_alphanumeric() || bytes[i - 1] == b'_' || bytes[i - 1] == b'.'); + let start = i; + while i < bytes.len() && (bytes[i].is_ascii_alphanumeric() || bytes[i] == b'_') { + i += 1; + } + debug_assert!( + !text[start..i].contains('\n'), + "a token must not span a newline, or the line counter is wrong" + ); + if prev_is_word { + continue; + } + let tok = &text[start..i]; + // Hex, binary and anything with a letter in it is left alone: mutating + // `0x3FCF1BBD` by +1 is meaningful, but `1e12` and `0b10` are not + // reliably parsed here, and a wrong mutant wastes a whole run. + let (from, to) = if let Some(h) = tok.strip_prefix("0x").or_else(|| tok.strip_prefix("0X")) { + match u64::from_str_radix(h, 16) { + Ok(v) => (tok.to_string(), format!("0x{:X}", v.wrapping_add(1))), + Err(_) => continue, + } + } else { + match tok.parse::() { + Ok(v) => (tok.to_string(), (v + 1).to_string()), + Err(_) => continue, + } + }; + let line_start = text[..start].rfind('\n').map(|p| p + 1).unwrap_or(0); + out.push(Mutant { + line, + col: start - line_start + 1, + from, + to, + byte: start, + len: tok.len(), + }); + } + out +} + +fn passes(cmd: &str) -> Result { + let out = Command::new("sh") + .arg("-c") + .arg(cmd) + .output() + .context("failed to run the checker command")?; + Ok(out.status.success()) +} + +fn mutate(file: &Path, cmd: &str, max: usize) -> Result<()> { + let original = std::fs::read_to_string(file) + .with_context(|| format!("cannot read {}", file.display()))?; + let backup = make_recoverable(file, &original)?; + + // A checker that is already failing cannot tell us anything about a + // mutant: every mutant would "survive" by looking exactly like the + // baseline. Establish the baseline before changing a byte. + if !passes(cmd)? { + bail!( + "the checker does not pass on the unmodified file, so no mutant \ + would mean anything. Fix it first, then run this." + ); + } + + let mutants = find_mutants(&original, max); + if mutants.is_empty() { + println!("No numeric literals found in {}.", file.display()); + return Ok(()); + } + + println!( + "{} literal(s) in {}, one mutation each.\n", + mutants.len(), + file.display() + ); + + let mut survivors = Vec::new(); + for (n, m) in mutants.iter().enumerate() { + let mut text = String::with_capacity(original.len()); + text.push_str(&original[..m.byte]); + text.push_str(&m.to); + text.push_str(&original[m.byte + m.len..]); + std::fs::write(file, &text)?; + let survived = passes(cmd).unwrap_or(false); + std::fs::write(file, &original)?; + clear_derived_caches(file); + + // Verify the restore instead of assuming it. A measurement taken + // against a file this command left perturbed is not a measurement, and + // that is not hypothetical: a perturbed constant survived a hand-run + // mutation on this machine, was read back as if it were the real value, + // and produced a written-up finding that did not exist. Failing loudly + // here costs one read per mutant and makes that silent. + let back = std::fs::read_to_string(file) + .with_context(|| format!("cannot re-read {} after restoring it", file.display()))?; + if back != original { + bail!( + "{} was NOT restored after mutating line {}. Recover it from {} \ + before trusting any measurement taken against it.", + file.display(), + m.line, + backup.display() + ); + } + + print!("\r {}/{} ", n + 1, mutants.len()); + use std::io::Write; + let _ = std::io::stdout().flush(); + + if survived { + survivors.push(m); + } + } + println!("\r "); + + clear_derived_caches(file); + let _ = std::fs::remove_file(&backup); + if survivors.is_empty() { + println!( + "Every one of the {} literals changed the outcome. Nothing in this \ + file is decorative.", + mutants.len() + ); + return Ok(()); + } + + println!( + "{} of {} mutations SURVIVED — the checker did not notice:\n", + survivors.len(), + mutants.len() + ); + for m in &survivors { + println!( + " {}:{}:{} {} -> {}", + file.display(), + m.line, + m.col, + m.from, + m.to + ); + } + println!(); + println!("A survivor is a question, not a verdict: some constants genuinely"); + println!("do not affect the outcome. But a check that cannot fail is"); + println!("indistinguishable from one that passed, so answer each of them."); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `gf16` and `sha1` are names, not constants. An early version of this + /// scanner mutated the `16` in `gf16` and produced a mutant that failed for + /// a reason unrelated to any check — noise that reads exactly like signal. + #[test] + fn digits_inside_an_identifier_are_not_literals() { + let m = find_mutants("let gf16 = 9;\nsha1 + 2\n", 20); + let got: Vec<&str> = m.iter().map(|x| x.from.as_str()).collect(); + assert_eq!(got, vec!["9", "2"], "identifier digits must be skipped"); + } + + /// Two identical literals on one line must be distinguishable, or a + /// survivor cannot be told from the one beside it that was caught. + #[test] + fn identical_literals_on_one_line_get_different_columns() { + let m = find_mutants("x = 5 * b > 5 * c\n", 20); + assert_eq!(m.len(), 2); + assert_eq!(m[0].line, m[1].line); + assert_ne!(m[0].col, m[1].col, "the columns must differ"); + assert_eq!(m[0].col, 5); + } + + #[test] + fn hex_is_mutated_as_hex() { + let m = find_mutants("WANT = 0x3FCF1BBD\n", 20); + assert_eq!(m.len(), 1); + assert_eq!(m[0].from, "0x3FCF1BBD"); + assert_eq!(m[0].to, "0x3FCF1BBE"); + } + + /// Numbers in prose cannot fail a check, so mutating them manufactures + /// false survivors. The first run of this command produced 26 of them and + /// one real result, which is a tool nobody would read twice. + #[test] + fn numbers_in_comments_and_strings_are_not_mutated() { + let src = "# phi^2 = phi + 1\nWANT = 7\nmsg = \"1 < phi < 2\"\n"; + let m = find_mutants(src, 20); + let got: Vec<&str> = m.iter().map(|x| x.from.as_str()).collect(); + assert_eq!(got, vec!["7"], "only the assignment is a real literal"); + } + + /// The line number is the only part of the report a human navigates by, so + /// it is counted rather than estimated. + #[test] + fn line_numbers_survive_multiline_input() { + let m = find_mutants("a\nb\n= 7\n", 20); + assert_eq!(m.len(), 1); + assert_eq!(m[0].line, 3); + } + + /// The test above uses input with no comments in it, so it cannot tell + /// whether masked regions advance the line counter. These two can. Added + /// after I wrongly accused the counter of losing newlines — the counter was + /// right, and an untested property that happens to hold is still untested. + #[test] + fn line_numbers_count_newlines_inside_comments_and_strings() { + let src = "x = 1\n# one\n# two\n# three\ny = 2\n"; + let m = find_mutants(src, 20); + let lines: Vec = m.iter().map(|x| x.line).collect(); + assert_eq!(lines, vec![1, 5], "the three comment lines must be counted"); + } + + #[test] + fn line_numbers_count_newlines_inside_a_docstring() { + let src = "a = 1\n\"\"\"\nline\nline\nline\n\"\"\"\nb = 2\n"; + let m = find_mutants(src, 20); + let lines: Vec = m.iter().map(|x| x.line).collect(); + assert_eq!(lines, vec![1, 7], "a multi-line docstring must be counted"); + } +} diff --git a/cli/tri/src/prcheck.rs b/cli/tri/src/prcheck.rs new file mode 100644 index 0000000000..b5ce658093 --- /dev/null +++ b/cli/tri/src/prcheck.rs @@ -0,0 +1,323 @@ +//! `tri pr ready` — is this pull request actually safe to merge? +//! +//! Written after merging a pull request whose language audit was red. The +//! failure was there, in the list, and I read a summary line I had written +//! myself instead of the list. The gate was correct; I was not. +//! +//! The judgement that matters is not "is anything failing" -- in these +//! repositories something is always failing -- but "is anything failing HERE +//! that is not already failing everywhere else". So this classifies every +//! failure against the default branch and against recently merged pull +//! requests, and prints one unambiguous verdict line at the end. +//! +//! It refuses to guess. A check whose status it cannot classify is reported as +//! unclassified and blocks the verdict, because an unread check is exactly what +//! this command exists to prevent. + +use anyhow::{Context, Result}; +use clap::Subcommand; +use std::collections::BTreeMap; +use std::process::Command; + +#[derive(Subcommand)] +pub enum PrCmd { + /// Classify every failing check and say plainly whether it is safe to merge. + Ready { + /// Pull request number. + number: u64, + /// owner/repo. Defaults to the repository in the current directory. + #[arg(long)] + repo: Option, + /// How many recently merged pull requests to compare against. + #[arg(long, default_value_t = 5)] + baseline: usize, + /// Block until every check has finished, then report. Without this a + /// verdict can be computed while checks are still starting. + #[arg(long)] + wait: bool, + /// Seconds between polls while waiting. + #[arg(long, default_value_t = 30)] + poll: u64, + /// Merge the pull request if — and only if — the verdict is safe. + /// + /// The verdict cannot gate anything if the caller puts `gh pr merge` in + /// the same batch as this command: it prints WAIT, the merge runs + /// anyway, and nobody reads the line. That happened four times in one + /// session. Handing the merge to the command makes the two inseparable. + #[arg(long)] + merge: bool, + }, +} + +pub fn run(cmd: &PrCmd) -> Result<()> { + match cmd { + PrCmd::Ready { + number, + repo, + baseline, + wait, + poll, + merge, + } => ready(*number, repo.as_deref(), *baseline, *wait, *poll, *merge), + } +} + +fn gh(args: &[&str]) -> Result { + let out = Command::new("gh") + .args(args) + .output() + .context("gh is not installed or not on PATH")?; + if !out.status.success() { + anyhow::bail!( + "gh {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr).trim() + ); + } + Ok(String::from_utf8_lossy(&out.stdout).trim().to_string()) +} + +/// Names of checks that failed on a given pull request. +fn failures_of(repo: &str, n: u64) -> Result> { + let raw = gh(&[ + "api", + &format!("repos/{repo}/pulls/{n}"), + "--jq", + ".head.sha", + ])?; + let sha = raw.trim(); + let runs = gh(&[ + "api", + &format!("repos/{repo}/commits/{sha}/check-runs?per_page=100"), + "--jq", + r#".check_runs[]|select(.conclusion=="failure"or .conclusion=="timed_out")|.name"#, + ])?; + // A name can appear on several check-runs (matrix entries, re-runs), and + // printing it twice makes a short list look like a long one. + let mut names: Vec = runs.lines().map(|s| s.to_string()).collect(); + names.sort(); + names.dedup(); + Ok(names) +} + +/// Number of checks not yet completed on this pull request's head. +/// +/// Zero can mean two different things and only one of them is "finished": no +/// checks have STARTED yet also reports zero. That is not hypothetical -- a +/// polling loop of mine exited on an empty list, and the pull request was +/// merged while ten checks were still running. So this returns the completed +/// count too, and the caller waits for it to stop growing. +fn in_flight(repo: &str, n: u64) -> Result<(usize, usize)> { + let sha = gh(&["api", &format!("repos/{repo}/pulls/{n}"), "--jq", ".head.sha"])?; + // Two plain queries rather than one clever @tsv: the combined form failed + // with "expected an object but got: array" the first time it ran, and a + // wait loop that errors out is worse than no wait loop. + let path = format!("repos/{repo}/commits/{}/check-runs?per_page=100", sha.trim()); + let pending: usize = gh(&[ + "api", + &path, + "--jq", + r#"[.check_runs[]|select(.status!="completed")]|length"#, + ])? + .trim() + .parse() + .unwrap_or(0); + let total: usize = gh(&["api", &path, "--jq", ".check_runs|length"])? + .trim() + .parse() + .unwrap_or(0); + Ok((pending, total)) +} + +fn ready(n: u64, repo: Option<&str>, baseline: usize, wait: bool, poll: u64, merge: bool) -> Result<()> { + let repo = match repo { + Some(r) => r.to_string(), + None => gh(&["repo", "view", "--json", "nameWithOwner", "--jq", ".nameWithOwner"])?, + }; + + // Anything still running makes the answer provisional, so say so rather + // than reporting a verdict on a partial list. + let mut pending = in_flight(&repo, n)?.0; + if wait { + let mut quiet = 0; + let mut blips = 0; + loop { + // A transient API failure must not end the wait. The first time + // this loop met a TLS handshake timeout it propagated the error, + // the caller's merge ran anyway, and the gate protected nothing -- + // the third time in this project that a verdict failed to gate. + let (p, total) = match in_flight(&repo, n) { + Ok(v) => { + blips = 0; + v + } + Err(e) => { + blips += 1; + if blips > 5 { + return Err(e).context( + "the check API failed six times running; refusing to \ + report a verdict rather than guess at the state", + ); + } + println!(" waiting: check API failed ({blips}/5), retrying"); + std::thread::sleep(std::time::Duration::from_secs(poll)); + continue; + } + }; + if p > 0 { + quiet = 0; + println!(" waiting: {p} of {total} check(s) still running"); + } else if total == 0 { + // An empty list is not "finished" -- it is "not started". Give + // it a few rounds before believing it. + quiet += 1; + println!(" waiting: no checks have appeared yet ({quiet}/4)"); + if quiet >= 4 { + break; + } + } else { + break; + } + std::thread::sleep(std::time::Duration::from_secs(poll)); + } + pending = match in_flight(&repo, n) { + Ok(v) => v.0, + // Unknown is not zero. If the final read fails, say so and let the + // verdict be WAIT rather than inventing a clean list. + Err(_) => 1, + }; + println!(); + } + + let mine = failures_of(&repo, n)?; + + // The baseline: failures on the default branch, plus failures on the last + // few merged pull requests. A check red in both places is the repository's + // problem, not this change's. + let branch = gh(&["api", &format!("repos/{repo}"), "--jq", ".default_branch"])?; + let head = gh(&[ + "api", + &format!("repos/{repo}/commits/{branch}"), + "--jq", + ".sha", + ])?; + let mut seen: BTreeMap = BTreeMap::new(); + for name in gh(&[ + "api", + &format!("repos/{repo}/commits/{}/check-runs?per_page=100", head.trim()), + "--jq", + r#".check_runs[]|select(.conclusion=="failure")|.name"#, + ])? + .lines() + { + *seen.entry(name.to_string()).or_insert(0) += 1; + } + let merged = gh(&[ + "api", + &format!("repos/{repo}/pulls?state=closed&per_page={}", baseline * 3), + "--jq", + ".[]|select(.merged_at!=null)|.number", + ])?; + for num in merged.lines().take(baseline) { + if let Ok(p) = num.parse::() { + if p == n { + continue; + } + for name in failures_of(&repo, p).unwrap_or_default() { + *seen.entry(name).or_insert(0) += 1; + } + } + } + + println!("{repo}#{n}\n"); + if mine.is_empty() { + println!(" nothing is failing"); + } + let mut new_here = Vec::new(); + for name in &mine { + match seen.get(name) { + Some(k) => println!(" {name}\n also failing in {k} other place(s) — pre-existing"), + None => { + println!(" {name}\n NOT failing on {branch} or in the last {baseline} merged PRs"); + new_here.push(name.clone()); + } + } + } + println!(); + if pending > 0 { + println!("VERDICT: WAIT — {pending} check(s) still running, the list is incomplete."); + if merge { + println!("Not merging: the list is incomplete. Re-run with --wait."); + } + } else if new_here.is_empty() { + println!("VERDICT: safe to merge — every failure is failing elsewhere too."); + if merge { + println!(); + let out = Command::new("gh") + .args(["pr", "merge", &n.to_string(), "--repo", &repo, + "--squash", "--delete-branch"]) + .output() + .context("failed to run gh pr merge")?; + if out.status.success() { + println!("Merged."); + } else { + println!("Merge refused: {}", String::from_utf8_lossy(&out.stderr).trim()); + } + } + } else { + println!( + "VERDICT: DO NOT MERGE — {} failure(s) appear only here:", + new_here.len() + ); + for name in &new_here { + println!(" - {name}"); + } + println!("\nRead the log before deciding they are unrelated. A summary line"); + println!("is not the list; that mistake is why this command exists."); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + /// A failure appearing on the default branch and on merged pull requests is + /// the repository's, not this change's. A failure appearing only here is + /// this change's until a log says otherwise -- and the default has to be + /// "stop", because the cost of the two mistakes is not symmetric. + #[test] + fn only_failures_unique_to_this_pr_block_the_verdict() { + let mine = vec!["checks".to_string(), "claude-review".to_string()]; + let elsewhere = vec!["claude-review".to_string()]; + let new_here: Vec<_> = mine.iter().filter(|m| !elsewhere.contains(m)).collect(); + assert_eq!(new_here.len(), 1); + assert_eq!(new_here[0], "checks"); + } + + /// An empty check list means "not started", not "finished". A polling loop + /// of mine counted rows, saw none, called it done, and a pull request was + /// merged while ten checks were still running -- with this command's own + /// WAIT verdict printed in the same batch, unread. + #[test] + fn an_empty_check_list_is_not_finished() { + let total = 0usize; + let pending = 0usize; + let finished = total > 0 && pending == 0; + assert!(!finished, "zero of zero must not read as complete"); + } + + /// A verdict computed from a partial list is worse than no verdict: it + /// reads exactly like a complete one. + #[test] + fn pending_checks_produce_wait_not_safe() { + let pending = 3usize; + let new_here: Vec = vec![]; + let verdict = if pending > 0 { + "WAIT" + } else if new_here.is_empty() { + "safe" + } else { + "DO NOT MERGE" + }; + assert_eq!(verdict, "WAIT", "pending must outrank an empty failure list"); + } +} diff --git a/cli/tri/src/red.rs b/cli/tri/src/red.rs new file mode 100644 index 0000000000..a3d454e7f5 --- /dev/null +++ b/cli/tri/src/red.rs @@ -0,0 +1,199 @@ +//! `tri red` — what is failing on main right now, and since when. +//! +//! This exists because of a specific evening. The publisher for t27.ai failed +//! six consecutive times and the site served hours-old content. A watchdog +//! caught it correctly and went red five times, on my own commits. I did not +//! read it once. I found the outage by accident, hours later, when a page I +//! had just published returned 404. +//! +//! The detection was never the problem. Reading it was. So the point of this +//! command is that there is no longer an excuse: one call, every repository, +//! newest failure first, with how long each has been failing. +//! +//! It deliberately reports the LATEST run per workflow on the default branch +//! rather than a window average. "Is it broken now" and "how often does it +//! break" are different questions, and only the first one stops a deploy. + +use anyhow::{Context, Result}; +use clap::Subcommand; +use std::process::Command; + +#[derive(Subcommand)] +pub enum RedCmd { + /// Show workflows whose most recent run on the default branch failed. + Now { + /// owner/repo, repeatable. Defaults to the three this fleet uses. + #[arg(long = "repo")] + repos: Vec, + /// Include workflows whose latest run was cancelled or timed out. + #[arg(long)] + include_cancelled: bool, + }, +} + +pub fn run(cmd: &RedCmd) -> Result<()> { + match cmd { + RedCmd::Now { + repos, + include_cancelled, + } => { + let list: Vec = if repos.is_empty() { + [ + "gHashTag/trinity", + "gHashTag/ghashtag.github.io", + "gHashTag/trinity-fpga", + "gHashTag/t27", + ] + .iter() + .map(|s| s.to_string()) + .collect() + } else { + repos.clone() + }; + now(&list, *include_cancelled) + } + } +} + +fn gh(args: &[&str]) -> Result { + let out = Command::new("gh") + .args(args) + .output() + .context("gh is not installed or not on PATH")?; + if !out.status.success() { + anyhow::bail!( + "gh {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr).trim() + ); + } + Ok(String::from_utf8_lossy(&out.stdout).trim().to_string()) +} + +struct Red { + repo: String, + name: String, + since: String, + consecutive: usize, + /// True when the streak filled the page and the real run is at least this. + at_least: bool, +} + +/// How many of the most recent runs, newest first, share the failing verdict. +/// A single red is noise; nine in a row is an outage nobody is reading. +fn streak(repo: &str, id: &str, branch: &str) -> Result<(usize, String)> { + let raw = gh(&[ + "api", + &format!("repos/{repo}/actions/workflows/{id}/runs?branch={branch}&per_page=30"), + "--jq", + r#".workflow_runs[]|"\(.conclusion)\t\(.created_at)""#, + ])?; + let mut n = 0usize; + let mut since = String::new(); + for line in raw.lines() { + let mut it = line.splitn(2, '\t'); + let concl = it.next().unwrap_or(""); + let at = it.next().unwrap_or(""); + match concl { + // A skipped run is not a verdict about the code — it means the + // gate's condition was not met. Counting it either way is wrong, + // so it is stepped over. + "skipped" | "null" | "" => continue, + "failure" | "timed_out" | "cancelled" => { + n += 1; + since = at.to_string(); + } + _ => break, + } + } + // n is bounded by the page size above, so a full page is a LOWER BOUND and + // must not be printed as if it were exact. That is the same silent + // truncation this command exists to surface, and it appeared here first. + Ok((n, since)) +} + +fn now(repos: &[String], include_cancelled: bool) -> Result<()> { + let mut reds: Vec = Vec::new(); + for repo in repos { + let branch = gh(&["api", &format!("repos/{repo}"), "--jq", ".default_branch"])?; + let listing = gh(&[ + "api", + &format!("repos/{repo}/actions/workflows?per_page=100"), + "--jq", + r#".workflows[]|select(.state=="active")|"\(.id)\t\(.name)""#, + ])?; + for line in listing.lines() { + let mut it = line.splitn(2, '\t'); + let (id, name) = match (it.next(), it.next()) { + (Some(a), Some(b)) => (a, b), + _ => continue, + }; + let latest = gh(&[ + "api", + &format!("repos/{repo}/actions/workflows/{id}/runs?branch={branch}&per_page=1"), + "--jq", + r#".workflow_runs[0].conclusion // "none""#, + ])?; + let bad = latest == "failure" + || latest == "timed_out" + || (include_cancelled && latest == "cancelled"); + if !bad { + continue; + } + let (n, since) = streak(repo, id, &branch)?; + reds.push(Red { + repo: repo.clone(), + name: name.to_string(), + since: since.chars().take(16).collect(), + consecutive: n, + at_least: n >= 30, + }); + } + } + + if reds.is_empty() { + println!("Nothing is red on the default branch of any of these repositories."); + return Ok(()); + } + + reds.sort_by(|a, b| b.consecutive.cmp(&a.consecutive)); + println!("{} workflow(s) red on the default branch:\n", reds.len()); + for r in &reds { + let short: String = r.name.chars().take(38).collect(); + let count = if r.at_least { + format!("{}+", r.consecutive) + } else { + r.consecutive.to_string() + }; + println!( + " {:>5} in a row since {} {:<26} {}", + count, r.since, r.repo, short + ); + } + println!(); + println!("A long streak is not more of the same failure — it is the number of"); + println!("times nobody looked. Read this before merging, not after a page 404s."); + Ok(()) +} + +#[cfg(test)] +mod tests { + /// `skipped` is not a verdict about the code: it means the workflow's own + /// condition was not met. Counting it as a success would end a real streak + /// early; counting it as a failure would invent one. Both were live + /// mistakes here — a run polled for its result had been skipped, and + /// `completed` was matched without looking at the conclusion. + #[test] + fn skipped_is_not_a_verdict_in_either_direction() { + let seq = ["failure", "skipped", "failure", "success"]; + let mut n = 0; + for c in seq { + match c { + "skipped" => continue, + "failure" => n += 1, + _ => break, + } + } + assert_eq!(n, 2, "the skipped run must neither end nor extend the streak"); + } +} diff --git a/cli/tri/src/rtl.rs b/cli/tri/src/rtl.rs new file mode 100644 index 0000000000..5589783c1d --- /dev/null +++ b/cli/tri/src/rtl.rs @@ -0,0 +1,357 @@ +//! `tri rtl` — the structural check t27.ai offers, run locally. +//! +//! This is the same five checks the reusable workflow performs +//! (`gHashTag/trinity .github/workflows/rtl-check.yml`), in one command, so a +//! design can be checked without pushing anything and a CI number can be +//! reproduced on demand. +//! +//! Two things it does that the hand-typed yosys invocation kept getting wrong: +//! +//! 1. **It names the instrument.** The cell count depends on which yosys ran. +//! Measured on one design with this exact script, 0.33 reports 45 cells +//! where 0.65 reports 49 — while wires and flip-flops agree exactly. A cell +//! count without its version is not reproducible, so the version is printed +//! beside it rather than left in the shell history. +//! +//! 2. **It counts its own verdicts.** Five checks must emit five lines. A +//! check that silently does not run looks exactly like one that passed, and +//! that has happened here before — so the count is asserted, not assumed. + +use anyhow::{bail, Context, Result}; +use clap::Subcommand; +use std::path::{Path, PathBuf}; +use std::process::Command; + +#[derive(Subcommand)] +pub enum RtlCmd { + /// Run the five structural checks on a design directory. + Check { + /// Directory holding info.yaml and src/ (defaults to the current one). + #[arg(default_value = ".")] + path: String, + /// Top module. Read from info.yaml when omitted. + #[arg(long)] + top: Option, + /// Emit JSON instead of the human report. + #[arg(long)] + json: bool, + /// Fail if the flip-flop count differs from this. + #[arg(long)] + expect_flops: Option, + }, +} + +struct Verdict { + name: &'static str, + pass: bool, + detail: String, + command: String, +} + +pub fn run(cmd: &RtlCmd) -> Result<()> { + match cmd { + RtlCmd::Check { + path, + top, + json, + expect_flops, + } => check(Path::new(path), top.as_deref(), *json, *expect_flops), + } +} + +/// yosys prints its banner on stdout for `-V`; an absent tool is a hard error +/// rather than an empty string, because an empty version silently becomes an +/// unlabelled number in the report. +fn yosys_version() -> Result { + let out = Command::new("yosys") + .arg("-V") + .output() + .context("yosys is not installed or not on PATH")?; + let s = String::from_utf8_lossy(&out.stdout).to_string(); + let v = s + .split_whitespace() + .take(2) + .collect::>() + .join(" "); + if v.trim().is_empty() { + bail!("yosys -V printed nothing; refusing to report numbers from an unnamed tool"); + } + Ok(v) +} + +/// The `source_files:` list from info.yaml, as paths under `/src/`. +/// +/// Deliberately a small hand parser rather than a YAML dependency: the block is +/// a flat list of quoted scalars, and the failure mode that matters — a file +/// declared but absent — is about the filesystem, not about YAML. +fn declared_sources(dir: &Path) -> Result> { + let info = dir.join("info.yaml"); + let text = std::fs::read_to_string(&info) + .with_context(|| format!("no info.yaml at {}", info.display()))?; + let mut out = Vec::new(); + let mut in_block = false; + for line in text.lines() { + let t = line.trim(); + if t.starts_with("source_files:") { + in_block = true; + continue; + } + if in_block { + if let Some(rest) = t.strip_prefix("- ") { + let f = rest.trim().trim_matches('"').trim_matches('\''); + if !f.is_empty() { + out.push(dir.join("src").join(f)); + } + continue; + } + if !t.is_empty() && !t.starts_with('#') { + break; + } + } + } + if out.is_empty() { + bail!("info.yaml declares no source_files"); + } + Ok(out) +} + +fn top_from_info(dir: &Path) -> Result { + let text = std::fs::read_to_string(dir.join("info.yaml"))?; + for line in text.lines() { + let t = line.trim(); + if let Some(rest) = t.strip_prefix("top_module:") { + let v = rest.trim().trim_matches('"').trim_matches('\''); + if !v.is_empty() { + return Ok(v.to_string()); + } + } + } + bail!("info.yaml has no top_module and --top was not given") +} + +/// Any cell type containing "dff" is a flop, and yosys prints the count in +/// either field order depending on version — so take whichever field is a +/// number rather than assuming a column. +fn count_flops(stat: &str) -> u64 { + let mut total = 0u64; + for line in stat.lines() { + let low = line.to_lowercase(); + if !low.contains("dff") { + continue; + } + let mut fields = line.split_whitespace(); + let a = fields.next().unwrap_or(""); + let b = fields.next().unwrap_or(""); + if let Ok(n) = a.parse::() { + total += n; + } else if let Ok(n) = b.parse::() { + total += n; + } + } + total +} + +fn count_named(stat: &str, what: &str) -> Option { + for line in stat.lines() { + let t = line.trim(); + if let Some(rest) = t.strip_suffix(what) { + if let Ok(n) = rest.trim().parse::() { + return Some(n); + } + } + if let Some(rest) = t.strip_prefix(&format!("Number of {what}:")) { + if let Ok(n) = rest.trim().parse::() { + return Some(n); + } + } + } + None +} + +fn check(dir: &Path, top: Option<&str>, json: bool, expect_flops: Option) -> Result<()> { + let version = yosys_version()?; + let top = match top { + Some(t) => t.to_string(), + None => top_from_info(dir)?, + }; + let sources = declared_sources(dir)?; + let mut v: Vec = Vec::new(); + + // 1. Every declared file is present. Stated on the way through, not only on + // failure: a check that speaks only when it fails leaves the reader + // unable to tell "checked and passed" from "never ran". + let missing: Vec = sources + .iter() + .filter(|p| !p.is_file()) + .map(|p| p.display().to_string()) + .collect(); + v.push(Verdict { + name: "sources resolve", + pass: missing.is_empty(), + detail: if missing.is_empty() { + format!("every file info.yaml declares is present ({} of them)", sources.len()) + } else { + format!("declared but absent: {}", missing.join(", ")) + }, + command: "info.yaml source_files -> src/".to_string(), + }); + + // 2/3/4/5. One yosys pass. flatten before stat, because per-module counts + // under-report anything hierarchical. + let read = sources + .iter() + .map(|p| format!("read_verilog -sv {}", p.display())) + .collect::>() + .join("\n"); + let script = format!( + "{read}\nhierarchy -top {top}\nproc; opt; fsm; opt; memory; opt\ntechmap; opt\nflatten; opt\nstat\nselect -assert-none t:$_DLATCH_* t:$_DLATCHSR_*\n" + ); + let sp = dir.join(".tri-rtl-check.ys"); + std::fs::write(&sp, &script)?; + let out = Command::new("yosys") + .arg("-s") + .arg(&sp) + .output() + .context("failed to run yosys")?; + let _ = std::fs::remove_file(&sp); + let stat = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + let ok = out.status.success(); + let cmd = format!("yosys -s