From bd2195835dfecf829d6d9c746cc1d223782a299e Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 12:13:59 -0400 Subject: [PATCH 01/33] Add cargo ci CODEOWNERS license check --- .github/workflows/ci.yml | 30 +++ tools/ci/README.md | 12 ++ tools/ci/src/codeowners_check.rs | 353 +++++++++++++++++++++++++++++++ tools/ci/src/main.rs | 11 + 4 files changed, 406 insertions(+) create mode 100644 tools/ci/src/codeowners_check.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38ba84da17f..475df098cd6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -503,6 +503,36 @@ jobs: path: ${{ github.workspace }}/target/cargo-timings/ retention-days: 30 + codeowners_check: + needs: [merge_queue_noop] + if: ${{ needs.merge_queue_noop.outputs.skip != 'true' }} + name: CODEOWNERS check + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + env: + CARGO_TARGET_DIR: ${{ github.workspace }}/target + RUST_BACKTRACE: full + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v4 + + - uses: dsherret/rust-toolchain-file@v1 + - name: Set default rust toolchain + run: rustup default $(rustup show active-toolchain | cut -d' ' -f1) + + - name: Cache Rust dependencies + uses: Swatinem/rust-cache@v2 + with: + workspaces: ${{ github.workspace }} + shared-key: spacetimedb + save-if: false + prefix-key: v1 + + - name: Run CODEOWNERS check + run: cargo ci codeowners-check + wasm_bindings: needs: [merge_queue_noop] if: ${{ needs.merge_queue_noop.outputs.skip != 'true' }} diff --git a/tools/ci/README.md b/tools/ci/README.md index 78147590f46..52fd256d577 100644 --- a/tools/ci/README.md +++ b/tools/ci/README.md @@ -216,6 +216,18 @@ Usage: global-json-policy - `--help`: Print help +### `codeowners-check` + +**Usage:** +```bash +Usage: codeowners-check [OPTIONS] +``` + +**Options:** + +- `--pr-number `: Pull request number to inspect for approval state +- `--help`: Print help + ### `publish-checks` **Usage:** diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs new file mode 100644 index 00000000000..b0a596ca14f --- /dev/null +++ b/tools/ci/src/codeowners_check.rs @@ -0,0 +1,353 @@ +use anyhow::{anyhow, bail, Context, Result}; +use duct::cmd; +use regex::Regex; +use serde_json::Value; +use std::collections::HashMap; +use std::env; +use std::path::Path; +use std::sync::OnceLock; + +const REQUIRED_LICENSE_REVIEWER: &str = "cloutiertyler"; + +pub fn run(pr_number: Option) -> Result<()> { + super::ensure_repo_root()?; + + let base_ref = base_ref()?; + fetch_base_ref(&base_ref)?; + + let license_files = changed_license_files(&base_ref)?; + if license_files.is_empty() { + println!("No LICENSE files changed."); + return Ok(()); + } + + let disallowed_files = disallowed_license_changes(&base_ref, &license_files)?; + if disallowed_files.is_empty() { + println!("LICENSE changes are limited to version numbers and change dates."); + return Ok(()); + } + + let pr_number = pr_number.or_else(pr_number_from_env).ok_or_else(|| { + anyhow!( + "LICENSE files have non-version/date changes, but no pull request number was provided. \ + Re-run with --pr-number or set GitHub Actions pull request context." + ) + })?; + + if has_required_approval(pr_number)? { + println!("LICENSE changes approved by {REQUIRED_LICENSE_REVIEWER} on PR #{pr_number}."); + return Ok(()); + } + + bail!( + "LICENSE files have changes beyond version numbers and change dates, and PR #{pr_number} \ + does not have an approval from {REQUIRED_LICENSE_REVIEWER}: {}", + disallowed_files + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(", ") + ); +} + +fn base_ref() -> Result { + if let Ok(base_ref) = env::var("GITHUB_BASE_REF") { + if !base_ref.is_empty() { + return Ok(format!("origin/{base_ref}")); + } + } + + if let Ok(event_path) = env::var("GITHUB_EVENT_PATH") { + let event = std::fs::read_to_string(event_path)?; + let event: Value = serde_json::from_str(&event)?; + if let Some(base_ref) = event + .pointer("/pull_request/base/ref") + .and_then(Value::as_str) + .filter(|base_ref| !base_ref.is_empty()) + { + return Ok(format!("origin/{base_ref}")); + } + } + + Ok("origin/master".to_string()) +} + +fn fetch_base_ref(base_ref: &str) -> Result<()> { + let Some(ref_name) = base_ref.strip_prefix("origin/") else { + return Ok(()); + }; + cmd!( + "git", + "fetch", + "--no-tags", + "--depth=1", + "origin", + &format!("{ref_name}:refs/remotes/origin/{ref_name}") + ) + .run() + .with_context(|| format!("failed to fetch base ref {base_ref}"))?; + Ok(()) +} + +fn changed_license_files(base_ref: &str) -> Result> { + let output = cmd!("git", "diff", "--name-only", &format!("{base_ref}...HEAD")) + .read() + .with_context(|| format!("failed to list changed files against {base_ref}"))?; + Ok(output + .lines() + .map(Path::new) + .filter(|path| is_license_file(path)) + .map(Path::to_path_buf) + .collect()) +} + +fn disallowed_license_changes(base_ref: &str, license_files: &[std::path::PathBuf]) -> Result> { + let mut disallowed = Vec::new(); + for path in license_files { + let diff = cmd!( + "git", + "diff", + "--unified=0", + "--no-ext-diff", + &format!("{base_ref}...HEAD"), + "--", + path + ) + .read() + .with_context(|| format!("failed to read diff for {}", path.display()))?; + if !diff_only_changes_license_version_or_date(&diff) { + disallowed.push(path.clone()); + } + } + Ok(disallowed) +} + +fn is_license_file(path: &Path) -> bool { + let path_str = path.to_string_lossy(); + if path_str.starts_with("licenses/") { + return true; + } + + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.to_ascii_lowercase().starts_with("license")) +} + +fn diff_only_changes_license_version_or_date(diff: &str) -> bool { + let mut pending_removal: Option<&str> = None; + let mut saw_change = false; + + for line in diff.lines() { + if line.starts_with("diff --git ") + || line.starts_with("index ") + || line.starts_with("--- ") + || line.starts_with("+++ ") + || line.starts_with("@@") + { + continue; + } + + if let Some(removed) = line.strip_prefix('-') { + if pending_removal.replace(removed).is_some() { + return false; + } + saw_change = true; + continue; + } + + if let Some(added) = line.strip_prefix('+') { + let Some(removed) = pending_removal.take() else { + return false; + }; + if !allowed_license_line_change(removed, added) { + return false; + } + saw_change = true; + continue; + } + + if pending_removal.is_some() { + return false; + } + } + + pending_removal.is_none() && saw_change +} + +fn allowed_license_line_change(old: &str, new: &str) -> bool { + normalized_license_version_line(old) == normalized_license_version_line(new) + && normalized_license_version_line(old) != old + || normalized_change_date_line(old) == normalized_change_date_line(new) + && normalized_change_date_line(old) != old +} + +fn normalized_license_version_line(line: &str) -> String { + static VERSION: OnceLock = OnceLock::new(); + VERSION + .get_or_init(|| Regex::new(r"\bSpacetimeDB \d+\.\d+\.\d+\b").unwrap()) + .replace_all(line, "SpacetimeDB ") + .into_owned() +} + +fn normalized_change_date_line(line: &str) -> String { + static CHANGE_DATE: OnceLock = OnceLock::new(); + CHANGE_DATE + .get_or_init(|| Regex::new(r"\bChange Date:\s+\d{4}-\d{2}-\d{2}\b").unwrap()) + .replace_all(line, "Change Date: ") + .into_owned() +} + +fn pr_number_from_env() -> Option { + if let Ok(event_path) = env::var("GITHUB_EVENT_PATH") { + let event = std::fs::read_to_string(event_path).ok()?; + let event: Value = serde_json::from_str(&event).ok()?; + if let Some(number) = event.pointer("/pull_request/number").and_then(Value::as_u64) { + return Some(number); + } + if let Some(number) = event.pointer("/number").and_then(Value::as_u64) { + return Some(number); + } + if let Some(number) = event + .pointer("/inputs/pr_number") + .and_then(Value::as_str) + .and_then(|number| number.parse().ok()) + { + return Some(number); + } + if let Some(number) = event + .pointer("/merge_group/head_ref") + .and_then(Value::as_str) + .and_then(parse_pr_number_from_merge_group_ref) + { + return Some(number); + } + } + + for env_var in ["GITHUB_REF_NAME", "GITHUB_REF"] { + let Ok(value) = env::var(env_var) else { + continue; + }; + if let Some(number) = parse_pr_number_from_ref(&value) { + return Some(number); + } + } + + None +} + +fn parse_pr_number_from_ref(value: &str) -> Option { + static PR_REF: OnceLock = OnceLock::new(); + PR_REF + .get_or_init(|| Regex::new(r"(?:refs/pull/|^)(\d+)/(?:merge|head)$").unwrap()) + .captures(value) + .and_then(|captures| captures.get(1)) + .and_then(|number| number.as_str().parse().ok()) +} + +fn parse_pr_number_from_merge_group_ref(value: &str) -> Option { + static MERGE_GROUP_PR_REF: OnceLock = OnceLock::new(); + MERGE_GROUP_PR_REF + .get_or_init(|| Regex::new(r"/pr-(\d+)-").unwrap()) + .captures(value) + .and_then(|captures| captures.get(1)) + .and_then(|number| number.as_str().parse().ok()) +} + +fn has_required_approval(pr_number: u64) -> Result { + let repo = env::var("GITHUB_REPOSITORY").unwrap_or_else(|_| "clockworklabs/SpacetimeDB".to_string()); + let reviews_json = cmd!( + "gh", + "api", + &format!("repos/{repo}/pulls/{pr_number}/reviews?per_page=100") + ) + .read() + .with_context(|| format!("failed to read reviews for PR #{pr_number}"))?; + let reviews: Value = serde_json::from_str(&reviews_json)?; + let reviews = reviews + .as_array() + .ok_or_else(|| anyhow!("GitHub reviews response was not an array"))?; + + let mut latest_review_by_user = HashMap::new(); + for review in reviews { + let Some(login) = review.pointer("/user/login").and_then(Value::as_str) else { + continue; + }; + let Some(state) = review.get("state").and_then(Value::as_str) else { + continue; + }; + latest_review_by_user.insert(login, state); + } + + Ok(latest_review_by_user + .get(REQUIRED_LICENSE_REVIEWER) + .is_some_and(|state| *state == "APPROVED")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn allows_version_and_change_date_updates() { + let diff = "\ +diff --git a/LICENSE.txt b/LICENSE.txt +index 111..222 100644 +--- a/LICENSE.txt ++++ b/LICENSE.txt +@@ -8 +8 @@ Licensor: Clockwork Laboratories, Inc. +-Licensed Work: SpacetimeDB 2.3.0 ++Licensed Work: SpacetimeDB 2.4.0 +@@ -24 +24 @@ Additional Use Grant: You may make use of the Licensed Work provided your +-Change Date: 2031-05-26 ++Change Date: 2031-06-01 +"; + + assert!(diff_only_changes_license_version_or_date(diff)); + } + + #[test] + fn rejects_unpaired_license_addition() { + let diff = "\ +diff --git a/LICENSE.txt b/LICENSE.txt +--- a/LICENSE.txt ++++ b/LICENSE.txt +@@ -1,0 +2 @@ ++New license term +"; + + assert!(!diff_only_changes_license_version_or_date(diff)); + } + + #[test] + fn rejects_non_version_license_edit() { + let diff = "\ +diff --git a/LICENSE.txt b/LICENSE.txt +--- a/LICENSE.txt ++++ b/LICENSE.txt +@@ -1 +1 @@ +-Old license term ++New license term +"; + + assert!(!diff_only_changes_license_version_or_date(diff)); + } + + #[test] + fn detects_license_files() { + assert!(is_license_file(Path::new("LICENSE.txt"))); + assert!(is_license_file(Path::new("crates/cli/LICENSE"))); + assert!(is_license_file(Path::new("licenses/BSL.txt"))); + assert!(!is_license_file(Path::new("crates/cli/Cargo.toml"))); + } + + #[test] + fn parses_pr_refs() { + assert_eq!(parse_pr_number_from_ref("refs/pull/123/merge"), Some(123)); + assert_eq!(parse_pr_number_from_ref("456/head"), Some(456)); + assert_eq!(parse_pr_number_from_ref("master"), None); + assert_eq!( + parse_pr_number_from_merge_group_ref("gh-readonly-queue/master/pr-789-abcdef"), + Some(789) + ); + } +} diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 76a629d774f..4065a04131b 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -13,6 +13,7 @@ const README_PATH: &str = "tools/ci/README.md"; mod ci_docs; mod cla_assistant; +mod codeowners_check; mod keynote_bench; mod smoketest; mod util; @@ -362,6 +363,12 @@ enum CiCmd { /// Verify that any non-root global.json files are symlinks to the root global.json. GlobalJsonPolicy, + /// Checks that sensitive CODEOWNERS-controlled files have the required approvals. + CodeownersCheck { + /// Pull request number to inspect for approval state. + #[arg(long)] + pr_number: Option, + }, /// Checks that publishable crates satisfy publish constraints. PublishChecks, /// Runs TypeScript workspace tests and template build checks. @@ -789,6 +796,10 @@ fn main() -> Result<()> { check_global_json_policy()?; } + Some(CiCmd::CodeownersCheck { pr_number }) => { + codeowners_check::run(pr_number)?; + } + Some(CiCmd::PublishChecks) => { run_publish_checks()?; } From 0358487c00b4bfd689c4824c0ab03a47f946e46c Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 13:26:51 -0400 Subject: [PATCH 02/33] Address CODEOWNERS check review --- .github/workflows/ci.yml | 18 +++- tools/ci/README.md | 2 +- tools/ci/src/codeowners_check.rs | 152 ++++++++----------------------- tools/ci/src/main.rs | 30 +++++- 4 files changed, 83 insertions(+), 119 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 475df098cd6..125f81dac35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -505,7 +505,7 @@ jobs: codeowners_check: needs: [merge_queue_noop] - if: ${{ needs.merge_queue_noop.outputs.skip != 'true' }} + if: ${{ needs.merge_queue_noop.outputs.skip != 'true' && github.event_name != 'push' }} name: CODEOWNERS check runs-on: ubuntu-latest permissions: @@ -531,7 +531,21 @@ jobs: prefix-key: v1 - name: Run CODEOWNERS check - run: cargo ci codeowners-check + env: + PR_NUMBER: ${{ github.event.pull_request.number || github.event.inputs.pr_number }} + MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} + run: | + set -euo pipefail + + if [ -z "${PR_NUMBER}" ] && [[ "${MERGE_GROUP_HEAD_REF}" =~ /pr-([0-9]+)- ]]; then + PR_NUMBER="${BASH_REMATCH[1]}" + fi + if [ -z "${PR_NUMBER}" ]; then + echo "Could not resolve pull request number for CODEOWNERS check." >&2 + exit 1 + fi + + cargo ci codeowners-check --pr-number "${PR_NUMBER}" wasm_bindings: needs: [merge_queue_noop] diff --git a/tools/ci/README.md b/tools/ci/README.md index 52fd256d577..2d009453846 100644 --- a/tools/ci/README.md +++ b/tools/ci/README.md @@ -220,7 +220,7 @@ Usage: global-json-policy **Usage:** ```bash -Usage: codeowners-check [OPTIONS] +Usage: codeowners-check --pr-number ``` **Options:** diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index b0a596ca14f..a66f3ee08eb 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -2,39 +2,25 @@ use anyhow::{anyhow, bail, Context, Result}; use duct::cmd; use regex::Regex; use serde_json::Value; -use std::collections::HashMap; use std::env; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::OnceLock; const REQUIRED_LICENSE_REVIEWER: &str = "cloutiertyler"; -pub fn run(pr_number: Option) -> Result<()> { +pub fn run(pr_number: u64) -> Result<()> { super::ensure_repo_root()?; let base_ref = base_ref()?; fetch_base_ref(&base_ref)?; - let license_files = changed_license_files(&base_ref)?; - if license_files.is_empty() { - println!("No LICENSE files changed."); - return Ok(()); - } - - let disallowed_files = disallowed_license_changes(&base_ref, &license_files)?; + let disallowed_files = disallowed_changes(&base_ref)?; if disallowed_files.is_empty() { - println!("LICENSE changes are limited to version numbers and change dates."); + println!("CODEOWNERS-controlled file changes are allowed."); return Ok(()); } - let pr_number = pr_number.or_else(pr_number_from_env).ok_or_else(|| { - anyhow!( - "LICENSE files have non-version/date changes, but no pull request number was provided. \ - Re-run with --pr-number or set GitHub Actions pull request context." - ) - })?; - - if has_required_approval(pr_number)? { + if approved_by(pr_number, REQUIRED_LICENSE_REVIEWER)? { println!("LICENSE changes approved by {REQUIRED_LICENSE_REVIEWER} on PR #{pr_number}."); return Ok(()); } @@ -89,39 +75,43 @@ fn fetch_base_ref(base_ref: &str) -> Result<()> { Ok(()) } -fn changed_license_files(base_ref: &str) -> Result> { +fn changed_files(base_ref: &str) -> Result> { let output = cmd!("git", "diff", "--name-only", &format!("{base_ref}...HEAD")) .read() .with_context(|| format!("failed to list changed files against {base_ref}"))?; - Ok(output - .lines() - .map(Path::new) - .filter(|path| is_license_file(path)) - .map(Path::to_path_buf) - .collect()) + Ok(output.lines().map(Path::new).map(Path::to_path_buf).collect()) } -fn disallowed_license_changes(base_ref: &str, license_files: &[std::path::PathBuf]) -> Result> { +fn disallowed_changes(base_ref: &str) -> Result> { let mut disallowed = Vec::new(); - for path in license_files { - let diff = cmd!( - "git", - "diff", - "--unified=0", - "--no-ext-diff", - &format!("{base_ref}...HEAD"), - "--", - path - ) - .read() - .with_context(|| format!("failed to read diff for {}", path.display()))?; - if !diff_only_changes_license_version_or_date(&diff) { - disallowed.push(path.clone()); + + for path in changed_files(base_ref)? { + if is_license_file(&path) { + check_license_file_changes(base_ref, &path, &mut disallowed)?; } } + Ok(disallowed) } +fn check_license_file_changes(base_ref: &str, path: &Path, disallowed: &mut Vec) -> Result<()> { + let diff = cmd!( + "git", + "diff", + "--unified=0", + "--no-ext-diff", + &format!("{base_ref}...HEAD"), + "--", + path + ) + .read() + .with_context(|| format!("failed to read diff for {}", path.display()))?; + if !diff_only_changes_license_version_or_date(&diff) { + disallowed.push(path.to_path_buf()); + } + Ok(()) +} + fn is_license_file(path: &Path) -> bool { let path_str = path.to_string_lossy(); if path_str.starts_with("licenses/") { @@ -197,63 +187,7 @@ fn normalized_change_date_line(line: &str) -> String { .into_owned() } -fn pr_number_from_env() -> Option { - if let Ok(event_path) = env::var("GITHUB_EVENT_PATH") { - let event = std::fs::read_to_string(event_path).ok()?; - let event: Value = serde_json::from_str(&event).ok()?; - if let Some(number) = event.pointer("/pull_request/number").and_then(Value::as_u64) { - return Some(number); - } - if let Some(number) = event.pointer("/number").and_then(Value::as_u64) { - return Some(number); - } - if let Some(number) = event - .pointer("/inputs/pr_number") - .and_then(Value::as_str) - .and_then(|number| number.parse().ok()) - { - return Some(number); - } - if let Some(number) = event - .pointer("/merge_group/head_ref") - .and_then(Value::as_str) - .and_then(parse_pr_number_from_merge_group_ref) - { - return Some(number); - } - } - - for env_var in ["GITHUB_REF_NAME", "GITHUB_REF"] { - let Ok(value) = env::var(env_var) else { - continue; - }; - if let Some(number) = parse_pr_number_from_ref(&value) { - return Some(number); - } - } - - None -} - -fn parse_pr_number_from_ref(value: &str) -> Option { - static PR_REF: OnceLock = OnceLock::new(); - PR_REF - .get_or_init(|| Regex::new(r"(?:refs/pull/|^)(\d+)/(?:merge|head)$").unwrap()) - .captures(value) - .and_then(|captures| captures.get(1)) - .and_then(|number| number.as_str().parse().ok()) -} - -fn parse_pr_number_from_merge_group_ref(value: &str) -> Option { - static MERGE_GROUP_PR_REF: OnceLock = OnceLock::new(); - MERGE_GROUP_PR_REF - .get_or_init(|| Regex::new(r"/pr-(\d+)-").unwrap()) - .captures(value) - .and_then(|captures| captures.get(1)) - .and_then(|number| number.as_str().parse().ok()) -} - -fn has_required_approval(pr_number: u64) -> Result { +fn approved_by(pr_number: u64, reviewer: &str) -> Result { let repo = env::var("GITHUB_REPOSITORY").unwrap_or_else(|_| "clockworklabs/SpacetimeDB".to_string()); let reviews_json = cmd!( "gh", @@ -267,20 +201,21 @@ fn has_required_approval(pr_number: u64) -> Result { .as_array() .ok_or_else(|| anyhow!("GitHub reviews response was not an array"))?; - let mut latest_review_by_user = HashMap::new(); + let mut latest_approval = false; for review in reviews { let Some(login) = review.pointer("/user/login").and_then(Value::as_str) else { continue; }; + if login != reviewer { + continue; + } let Some(state) = review.get("state").and_then(Value::as_str) else { continue; }; - latest_review_by_user.insert(login, state); + latest_approval = state == "APPROVED"; } - Ok(latest_review_by_user - .get(REQUIRED_LICENSE_REVIEWER) - .is_some_and(|state| *state == "APPROVED")) + Ok(latest_approval) } #[cfg(test)] @@ -339,15 +274,4 @@ diff --git a/LICENSE.txt b/LICENSE.txt assert!(is_license_file(Path::new("licenses/BSL.txt"))); assert!(!is_license_file(Path::new("crates/cli/Cargo.toml"))); } - - #[test] - fn parses_pr_refs() { - assert_eq!(parse_pr_number_from_ref("refs/pull/123/merge"), Some(123)); - assert_eq!(parse_pr_number_from_ref("456/head"), Some(456)); - assert_eq!(parse_pr_number_from_ref("master"), None); - assert_eq!( - parse_pr_number_from_merge_group_ref("gh-readonly-queue/master/pr-789-abcdef"), - Some(789) - ); - } } diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 4065a04131b..b5e16a1fa30 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -20,6 +20,22 @@ mod util; use util::ensure_repo_root; +const DEFAULT_CI_SUBCOMMANDS: &[&str] = &[ + "test", + "lint", + "wasm-bindings", + "dlls", + "smoketests", + "keynote-bench", + "update-flow", + "cli-docs", + "global-json-policy", + "publish-checks", + "typescript-test", + "version-upgrade-check", + "docs", +]; + /// On Windows, `pnpm` is installed as a `.cmd` shim which `CreateProcess` cannot /// find without going through the shell. Wrapping with `cmd /c` fixes this. /// On Unix, we invoke `pnpm` directly. @@ -367,7 +383,7 @@ enum CiCmd { CodeownersCheck { /// Pull request number to inspect for approval state. #[arg(long)] - pr_number: Option, + pr_number: u64, }, /// Checks that publishable crates satisfy publish constraints. PublishChecks, @@ -385,10 +401,20 @@ enum CiCmd { } fn run_all_clap_subcommands(skips: &[String]) -> Result<()> { - let subcmds = Cli::command() + let known_subcommands = Cli::command() .get_subcommands() .map(|sc| sc.get_name().to_string()) .collect::>(); + let subcmds = DEFAULT_CI_SUBCOMMANDS + .iter() + .map(|subcmd| subcmd.to_string()) + .collect::>(); + + for subcmd in &subcmds { + if !known_subcommands.contains(subcmd) { + bail!("default CI subcommand {subcmd:?} is not registered"); + } + } for subcmd in subcmds { if skips.contains(&subcmd) { From 2d9014b214bfeea22cf0e89d70f1214f01ef5adc Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:26:01 -0400 Subject: [PATCH 03/33] Group non-ci workflow commands --- .github/workflows/ci.yml | 4 +- .github/workflows/cla-gate.yml | 2 +- .github/workflows/retry-cla-assistant.yml | 2 +- tools/ci/README.md | 64 ++++++++++++------- tools/ci/src/ci_docs.rs | 2 +- tools/ci/src/main.rs | 77 +++++++++++------------ 6 files changed, 84 insertions(+), 67 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 125f81dac35..a250e9349e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -545,7 +545,7 @@ jobs: exit 1 fi - cargo ci codeowners-check --pr-number "${PR_NUMBER}" + cargo ci other-workflows codeowners-check --pr-number "${PR_NUMBER}" wasm_bindings: needs: [merge_queue_noop] @@ -772,7 +772,7 @@ jobs: run: rustup default $(rustup show active-toolchain | cut -d' ' -f1) - name: Check for docs change - run: cargo ci self-docs --check + run: cargo ci other-workflows self-docs --check cli_docs: needs: [merge_queue_noop] diff --git a/.github/workflows/cla-gate.yml b/.github/workflows/cla-gate.yml index 5d85deff704..95cec937108 100644 --- a/.github/workflows/cla-gate.yml +++ b/.github/workflows/cla-gate.yml @@ -37,7 +37,7 @@ jobs: const { execFileSync } = require("child_process"); function claStatus(args) { - const command = ["ci", "cla-assistant", "status", ...args]; + const command = ["ci", "other-workflows", "cla-assistant", "status", ...args]; core.info(`Running cargo ${command.join(" ")}`); const output = execFileSync("cargo", command, { encoding: "utf8", diff --git a/.github/workflows/retry-cla-assistant.yml b/.github/workflows/retry-cla-assistant.yml index 12f10da44c8..a95bd3336d5 100644 --- a/.github/workflows/retry-cla-assistant.yml +++ b/.github/workflows/retry-cla-assistant.yml @@ -77,6 +77,6 @@ jobs: run: | while read -r pr_number; do if [ -n "${pr_number}" ]; then - cargo ci cla-assistant retry --pr-number "${pr_number}" + cargo ci other-workflows cla-assistant retry --pr-number "${pr_number}" fi done < "${{ steps.prs.outputs.path }}" diff --git a/tools/ci/README.md b/tools/ci/README.md index 2d009453846..9f429b0f272 100644 --- a/tools/ci/README.md +++ b/tools/ci/README.md @@ -193,86 +193,97 @@ Usage: cli-docs [OPTIONS] - `--spacetime-path `: specify a custom path to the SpacetimeDB repository root (where the main Cargo.toml is located) - `--help`: Print help (see a summary with '-h') -### `self-docs` +### `global-json-policy` **Usage:** ```bash -Usage: self-docs [OPTIONS] +Usage: global-json-policy ``` **Options:** -- `--check`: Only check for changes, do not generate the docs -- `--help`: Print help (see a summary with '-h') +- `--help`: Print help -### `global-json-policy` +### `publish-checks` **Usage:** ```bash -Usage: global-json-policy +Usage: publish-checks ``` **Options:** - `--help`: Print help -### `codeowners-check` +### `typescript-test` **Usage:** ```bash -Usage: codeowners-check --pr-number +Usage: typescript-test ``` **Options:** -- `--pr-number `: Pull request number to inspect for approval state - `--help`: Print help -### `publish-checks` +### `version-upgrade-check` **Usage:** ```bash -Usage: publish-checks +Usage: version-upgrade-check ``` **Options:** - `--help`: Print help -### `typescript-test` +### `docs` **Usage:** ```bash -Usage: typescript-test +Usage: docs ``` **Options:** - `--help`: Print help -### `version-upgrade-check` +### `other-workflows` **Usage:** ```bash -Usage: version-upgrade-check +Usage: other-workflows ``` **Options:** - `--help`: Print help -### `docs` +#### `self-docs` **Usage:** ```bash -Usage: docs +Usage: self-docs [OPTIONS] +``` + +**Options:** + +- `--check`: Only check for changes, do not generate the docs +- `--help`: Print help (see a summary with '-h') + +#### `codeowners-check` + +**Usage:** +```bash +Usage: codeowners-check --pr-number ``` **Options:** +- `--pr-number `: Pull request number to inspect for approval state - `--help`: Print help -### `cla-assistant` +#### `cla-assistant` **Usage:** ```bash @@ -283,7 +294,7 @@ Usage: cla-assistant - `--help`: Print help -#### `retry` +##### `retry` **Usage:** ```bash @@ -296,7 +307,7 @@ Usage: retry [OPTIONS] --pr-number - `--repo `: Repository in `owner/name` form. Defaults to GITHUB_REPOSITORY - `--help`: Print help -#### `status` +##### `status` **Usage:** ```bash @@ -310,6 +321,17 @@ Usage: status [OPTIONS] <--pr |--sha > - `--repo `: Repository in `owner/name` form. Defaults to GITHUB_REPOSITORY - `--help`: Print help +##### `help` + +**Usage:** +```bash +Usage: help [COMMAND]... +``` + +**Options:** + +- `subcommand `: Print help for the subcommand(s) + #### `help` **Usage:** @@ -338,5 +360,5 @@ Usage: help [COMMAND]... This document is auto-generated by running: ```bash -cargo ci self-docs +cargo ci other-workflows self-docs ``` \ No newline at end of file diff --git a/tools/ci/src/ci_docs.rs b/tools/ci/src/ci_docs.rs index 3fadcdaf0cc..8be5f79136c 100644 --- a/tools/ci/src/ci_docs.rs +++ b/tools/ci/src/ci_docs.rs @@ -21,7 +21,7 @@ This document provides an overview of the `cargo ci` command-line tool, and docu This document is auto-generated by running: ```bash -cargo ci self-docs +cargo ci other-workflows self-docs ```", usage ) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index b5e16a1fa30..1babc50bc4f 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -20,21 +20,7 @@ mod util; use util::ensure_repo_root; -const DEFAULT_CI_SUBCOMMANDS: &[&str] = &[ - "test", - "lint", - "wasm-bindings", - "dlls", - "smoketests", - "keynote-bench", - "update-flow", - "cli-docs", - "global-json-policy", - "publish-checks", - "typescript-test", - "version-upgrade-check", - "docs", -]; +const OTHER_WORKFLOWS_SUBCOMMAND: &str = "other-workflows"; /// On Windows, `pnpm` is installed as a `.cmd` shim which `CreateProcess` cannot /// find without going through the shell. Wrapping with `cmd /c` fixes this. @@ -368,6 +354,26 @@ enum CiCmd { )] spacetime_path: Option, }, + /// Verify that any non-root global.json files are symlinks to the root global.json. + GlobalJsonPolicy, + /// Checks that publishable crates satisfy publish constraints. + PublishChecks, + /// Runs TypeScript workspace tests and template build checks. + TypescriptTest, + /// Verifies that the repository version upgrade tool still works. + VersionUpgradeCheck, + /// Builds the docs site. + Docs, + /// Workflows that are not part of ci.yml should live here. + OtherWorkflows { + #[command(subcommand)] + cmd: OtherWorkflowsCmd, + }, +} + +#[derive(Subcommand)] +enum OtherWorkflowsCmd { + /// Generates this cargo ci README and checks for changes. SelfDocs { #[arg( long, @@ -376,23 +382,12 @@ enum CiCmd { )] check: bool, }, - - /// Verify that any non-root global.json files are symlinks to the root global.json. - GlobalJsonPolicy, /// Checks that sensitive CODEOWNERS-controlled files have the required approvals. CodeownersCheck { /// Pull request number to inspect for approval state. #[arg(long)] pr_number: u64, }, - /// Checks that publishable crates satisfy publish constraints. - PublishChecks, - /// Runs TypeScript workspace tests and template build checks. - TypescriptTest, - /// Verifies that the repository version upgrade tool still works. - VersionUpgradeCheck, - /// Builds the docs site. - Docs, /// Interacts with CLA Assistant. ClaAssistant { #[command(subcommand)] @@ -401,22 +396,16 @@ enum CiCmd { } fn run_all_clap_subcommands(skips: &[String]) -> Result<()> { - let known_subcommands = Cli::command() + let subcmds = Cli::command() .get_subcommands() .map(|sc| sc.get_name().to_string()) .collect::>(); - let subcmds = DEFAULT_CI_SUBCOMMANDS - .iter() - .map(|subcmd| subcmd.to_string()) - .collect::>(); - - for subcmd in &subcmds { - if !known_subcommands.contains(subcmd) { - bail!("default CI subcommand {subcmd:?} is not registered"); - } - } for subcmd in subcmds { + if subcmd == OTHER_WORKFLOWS_SUBCOMMAND { + log::info!("skipping {subcmd} because it is not part of ci.yml"); + continue; + } if skips.contains(&subcmd) { log::info!("skipping {subcmd} as requested"); continue; @@ -801,14 +790,16 @@ fn main() -> Result<()> { } } - Some(CiCmd::SelfDocs { check }) => { + Some(CiCmd::OtherWorkflows { + cmd: OtherWorkflowsCmd::SelfDocs { check }, + }) => { let readme_content = ci_docs::generate_cli_docs(); let path = Path::new(README_PATH); if check { let existing = fs::read_to_string(path).unwrap_or_default(); if existing != readme_content { - bail!("README.md is out of date. Please run `cargo ci self-docs` to update it."); + bail!("README.md is out of date. Please run `cargo ci other-workflows self-docs` to update it."); } else { log::info!("README.md is up to date."); } @@ -822,7 +813,9 @@ fn main() -> Result<()> { check_global_json_policy()?; } - Some(CiCmd::CodeownersCheck { pr_number }) => { + Some(CiCmd::OtherWorkflows { + cmd: OtherWorkflowsCmd::CodeownersCheck { pr_number }, + }) => { codeowners_check::run(pr_number)?; } @@ -842,7 +835,9 @@ fn main() -> Result<()> { run_docs_build()?; } - Some(CiCmd::ClaAssistant { cmd }) => { + Some(CiCmd::OtherWorkflows { + cmd: OtherWorkflowsCmd::ClaAssistant { cmd }, + }) => { cla_assistant::run(cmd)?; } From ff12b87a69be4acbd6cad876dd1a0dd9023084bb Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:31:00 -0400 Subject: [PATCH 04/33] Address CODEOWNERS check PR context review --- .github/workflows/ci.yml | 19 ++++-------------- tools/ci/README.md | 3 ++- tools/ci/src/codeowners_check.rs | 34 +++++--------------------------- tools/ci/src/main.rs | 7 +++++-- 4 files changed, 16 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a250e9349e3..3f9d2b54987 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -505,7 +505,7 @@ jobs: codeowners_check: needs: [merge_queue_noop] - if: ${{ needs.merge_queue_noop.outputs.skip != 'true' && github.event_name != 'push' }} + if: ${{ needs.merge_queue_noop.outputs.skip != 'true' && github.event_name == 'pull_request' }} name: CODEOWNERS check runs-on: ubuntu-latest permissions: @@ -531,21 +531,10 @@ jobs: prefix-key: v1 - name: Run CODEOWNERS check - env: - PR_NUMBER: ${{ github.event.pull_request.number || github.event.inputs.pr_number }} - MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} run: | - set -euo pipefail - - if [ -z "${PR_NUMBER}" ] && [[ "${MERGE_GROUP_HEAD_REF}" =~ /pr-([0-9]+)- ]]; then - PR_NUMBER="${BASH_REMATCH[1]}" - fi - if [ -z "${PR_NUMBER}" ]; then - echo "Could not resolve pull request number for CODEOWNERS check." >&2 - exit 1 - fi - - cargo ci other-workflows codeowners-check --pr-number "${PR_NUMBER}" + cargo ci other-workflows codeowners-check \ + --base-ref "origin/${{ github.event.pull_request.base.ref }}" \ + --pr-number "${{ github.event.pull_request.number }}" wasm_bindings: needs: [merge_queue_noop] diff --git a/tools/ci/README.md b/tools/ci/README.md index 9f429b0f272..8821c50a90e 100644 --- a/tools/ci/README.md +++ b/tools/ci/README.md @@ -275,11 +275,12 @@ Usage: self-docs [OPTIONS] **Usage:** ```bash -Usage: codeowners-check --pr-number +Usage: codeowners-check --base-ref --pr-number ``` **Options:** +- `--base-ref `: Git ref to compare against, usually origin/ - `--pr-number `: Pull request number to inspect for approval state - `--help`: Print help diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index a66f3ee08eb..f260c4d22a0 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -2,19 +2,18 @@ use anyhow::{anyhow, bail, Context, Result}; use duct::cmd; use regex::Regex; use serde_json::Value; -use std::env; use std::path::{Path, PathBuf}; use std::sync::OnceLock; +const REPO: &str = "clockworklabs/SpacetimeDB"; const REQUIRED_LICENSE_REVIEWER: &str = "cloutiertyler"; -pub fn run(pr_number: u64) -> Result<()> { +pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { super::ensure_repo_root()?; - let base_ref = base_ref()?; - fetch_base_ref(&base_ref)?; + fetch_base_ref(base_ref)?; - let disallowed_files = disallowed_changes(&base_ref)?; + let disallowed_files = disallowed_changes(base_ref)?; if disallowed_files.is_empty() { println!("CODEOWNERS-controlled file changes are allowed."); return Ok(()); @@ -36,28 +35,6 @@ pub fn run(pr_number: u64) -> Result<()> { ); } -fn base_ref() -> Result { - if let Ok(base_ref) = env::var("GITHUB_BASE_REF") { - if !base_ref.is_empty() { - return Ok(format!("origin/{base_ref}")); - } - } - - if let Ok(event_path) = env::var("GITHUB_EVENT_PATH") { - let event = std::fs::read_to_string(event_path)?; - let event: Value = serde_json::from_str(&event)?; - if let Some(base_ref) = event - .pointer("/pull_request/base/ref") - .and_then(Value::as_str) - .filter(|base_ref| !base_ref.is_empty()) - { - return Ok(format!("origin/{base_ref}")); - } - } - - Ok("origin/master".to_string()) -} - fn fetch_base_ref(base_ref: &str) -> Result<()> { let Some(ref_name) = base_ref.strip_prefix("origin/") else { return Ok(()); @@ -188,11 +165,10 @@ fn normalized_change_date_line(line: &str) -> String { } fn approved_by(pr_number: u64, reviewer: &str) -> Result { - let repo = env::var("GITHUB_REPOSITORY").unwrap_or_else(|_| "clockworklabs/SpacetimeDB".to_string()); let reviews_json = cmd!( "gh", "api", - &format!("repos/{repo}/pulls/{pr_number}/reviews?per_page=100") + &format!("repos/{REPO}/pulls/{pr_number}/reviews?per_page=100") ) .read() .with_context(|| format!("failed to read reviews for PR #{pr_number}"))?; diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 1babc50bc4f..c9f6bd569c7 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -384,6 +384,9 @@ enum OtherWorkflowsCmd { }, /// Checks that sensitive CODEOWNERS-controlled files have the required approvals. CodeownersCheck { + /// Git ref to compare against, usually origin/. + #[arg(long)] + base_ref: String, /// Pull request number to inspect for approval state. #[arg(long)] pr_number: u64, @@ -814,9 +817,9 @@ fn main() -> Result<()> { } Some(CiCmd::OtherWorkflows { - cmd: OtherWorkflowsCmd::CodeownersCheck { pr_number }, + cmd: OtherWorkflowsCmd::CodeownersCheck { base_ref, pr_number }, }) => { - codeowners_check::run(pr_number)?; + codeowners_check::run(&base_ref, pr_number)?; } Some(CiCmd::PublishChecks) => { From b22879b331178974f4a96e1fae7d1f3101b3cc1c Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:33:07 -0400 Subject: [PATCH 05/33] Structure CODEOWNERS policy dispatch --- tools/ci/src/codeowners_check.rs | 53 ++++++++++++-------------------- 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index f260c4d22a0..d45bd357773 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -13,26 +13,14 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { fetch_base_ref(base_ref)?; - let disallowed_files = disallowed_changes(base_ref)?; - if disallowed_files.is_empty() { - println!("CODEOWNERS-controlled file changes are allowed."); - return Ok(()); - } - - if approved_by(pr_number, REQUIRED_LICENSE_REVIEWER)? { - println!("LICENSE changes approved by {REQUIRED_LICENSE_REVIEWER} on PR #{pr_number}."); - return Ok(()); + for path in changed_files(base_ref)? { + if is_license_file(&path) { + require_review_from(base_ref, &path, pr_number, REQUIRED_LICENSE_REVIEWER)?; + } } - bail!( - "LICENSE files have changes beyond version numbers and change dates, and PR #{pr_number} \ - does not have an approval from {REQUIRED_LICENSE_REVIEWER}: {}", - disallowed_files - .iter() - .map(|path| path.display().to_string()) - .collect::>() - .join(", ") - ); + println!("CODEOWNERS-controlled file changes are allowed."); + Ok(()) } fn fetch_base_ref(base_ref: &str) -> Result<()> { @@ -59,19 +47,7 @@ fn changed_files(base_ref: &str) -> Result> { Ok(output.lines().map(Path::new).map(Path::to_path_buf).collect()) } -fn disallowed_changes(base_ref: &str) -> Result> { - let mut disallowed = Vec::new(); - - for path in changed_files(base_ref)? { - if is_license_file(&path) { - check_license_file_changes(base_ref, &path, &mut disallowed)?; - } - } - - Ok(disallowed) -} - -fn check_license_file_changes(base_ref: &str, path: &Path, disallowed: &mut Vec) -> Result<()> { +fn require_review_from(base_ref: &str, path: &Path, pr_number: u64, reviewer: &str) -> Result<()> { let diff = cmd!( "git", "diff", @@ -83,10 +59,19 @@ fn check_license_file_changes(base_ref: &str, path: &Path, disallowed: &mut Vec< ) .read() .with_context(|| format!("failed to read diff for {}", path.display()))?; - if !diff_only_changes_license_version_or_date(&diff) { - disallowed.push(path.to_path_buf()); + if diff_only_changes_license_version_or_date(&diff) { + return Ok(()); } - Ok(()) + + if approved_by(pr_number, reviewer)? { + return Ok(()); + } + + bail!( + "{} has changes beyond version numbers and change dates, and PR #{pr_number} \ + does not have an approval from {reviewer}", + path.display() + ); } fn is_license_file(path: &Path) -> bool { From 51c1b1f2b1ae1570909bda8d9d22dfdf48a356af Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:34:07 -0400 Subject: [PATCH 06/33] Keep CODEOWNERS check pull-request only --- .github/workflows/ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3f9d2b54987..dc50a21625c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -504,8 +504,7 @@ jobs: retention-days: 30 codeowners_check: - needs: [merge_queue_noop] - if: ${{ needs.merge_queue_noop.outputs.skip != 'true' && github.event_name == 'pull_request' }} + if: ${{ github.event_name == 'pull_request' }} name: CODEOWNERS check runs-on: ubuntu-latest permissions: From 0c145a98f210eeb02adba758c0d9f4c71d8fa17e Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:36:13 -0400 Subject: [PATCH 07/33] Group non-ci cargo ci workflow commands --- .github/workflows/ci.yml | 2 +- .github/workflows/cla-gate.yml | 2 +- .github/workflows/retry-cla-assistant.yml | 2 +- tools/ci/README.md | 54 ++++++++++++++++------- tools/ci/src/ci_docs.rs | 2 +- tools/ci/src/main.rs | 43 +++++++++++++----- 6 files changed, 73 insertions(+), 32 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38ba84da17f..4fb638cf6b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -728,7 +728,7 @@ jobs: run: rustup default $(rustup show active-toolchain | cut -d' ' -f1) - name: Check for docs change - run: cargo ci self-docs --check + run: cargo ci other-workflows self-docs --check cli_docs: needs: [merge_queue_noop] diff --git a/.github/workflows/cla-gate.yml b/.github/workflows/cla-gate.yml index 5d85deff704..95cec937108 100644 --- a/.github/workflows/cla-gate.yml +++ b/.github/workflows/cla-gate.yml @@ -37,7 +37,7 @@ jobs: const { execFileSync } = require("child_process"); function claStatus(args) { - const command = ["ci", "cla-assistant", "status", ...args]; + const command = ["ci", "other-workflows", "cla-assistant", "status", ...args]; core.info(`Running cargo ${command.join(" ")}`); const output = execFileSync("cargo", command, { encoding: "utf8", diff --git a/.github/workflows/retry-cla-assistant.yml b/.github/workflows/retry-cla-assistant.yml index 12f10da44c8..a95bd3336d5 100644 --- a/.github/workflows/retry-cla-assistant.yml +++ b/.github/workflows/retry-cla-assistant.yml @@ -77,6 +77,6 @@ jobs: run: | while read -r pr_number; do if [ -n "${pr_number}" ]; then - cargo ci cla-assistant retry --pr-number "${pr_number}" + cargo ci other-workflows cla-assistant retry --pr-number "${pr_number}" fi done < "${{ steps.prs.outputs.path }}" diff --git a/tools/ci/README.md b/tools/ci/README.md index 78147590f46..98b3b1d627b 100644 --- a/tools/ci/README.md +++ b/tools/ci/README.md @@ -193,18 +193,6 @@ Usage: cli-docs [OPTIONS] - `--spacetime-path `: specify a custom path to the SpacetimeDB repository root (where the main Cargo.toml is located) - `--help`: Print help (see a summary with '-h') -### `self-docs` - -**Usage:** -```bash -Usage: self-docs [OPTIONS] -``` - -**Options:** - -- `--check`: Only check for changes, do not generate the docs -- `--help`: Print help (see a summary with '-h') - ### `global-json-policy` **Usage:** @@ -260,7 +248,30 @@ Usage: docs - `--help`: Print help -### `cla-assistant` +### `other-workflows` + +**Usage:** +```bash +Usage: other-workflows +``` + +**Options:** + +- `--help`: Print help + +#### `self-docs` + +**Usage:** +```bash +Usage: self-docs [OPTIONS] +``` + +**Options:** + +- `--check`: Only check for changes, do not generate the docs +- `--help`: Print help (see a summary with '-h') + +#### `cla-assistant` **Usage:** ```bash @@ -271,7 +282,7 @@ Usage: cla-assistant - `--help`: Print help -#### `retry` +##### `retry` **Usage:** ```bash @@ -284,7 +295,7 @@ Usage: retry [OPTIONS] --pr-number - `--repo `: Repository in `owner/name` form. Defaults to GITHUB_REPOSITORY - `--help`: Print help -#### `status` +##### `status` **Usage:** ```bash @@ -298,6 +309,17 @@ Usage: status [OPTIONS] <--pr |--sha > - `--repo `: Repository in `owner/name` form. Defaults to GITHUB_REPOSITORY - `--help`: Print help +##### `help` + +**Usage:** +```bash +Usage: help [COMMAND]... +``` + +**Options:** + +- `subcommand `: Print help for the subcommand(s) + #### `help` **Usage:** @@ -326,5 +348,5 @@ Usage: help [COMMAND]... This document is auto-generated by running: ```bash -cargo ci self-docs +cargo ci other-workflows self-docs ``` \ No newline at end of file diff --git a/tools/ci/src/ci_docs.rs b/tools/ci/src/ci_docs.rs index 3fadcdaf0cc..8be5f79136c 100644 --- a/tools/ci/src/ci_docs.rs +++ b/tools/ci/src/ci_docs.rs @@ -21,7 +21,7 @@ This document provides an overview of the `cargo ci` command-line tool, and docu This document is auto-generated by running: ```bash -cargo ci self-docs +cargo ci other-workflows self-docs ```", usage ) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 76a629d774f..b0dee4ccebb 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -19,6 +19,8 @@ mod util; use util::ensure_repo_root; +const OTHER_WORKFLOWS_SUBCOMMAND: &str = "other-workflows"; + /// On Windows, `pnpm` is installed as a `.cmd` shim which `CreateProcess` cannot /// find without going through the shell. Wrapping with `cmd /c` fixes this. /// On Unix, we invoke `pnpm` directly. @@ -351,15 +353,6 @@ enum CiCmd { )] spacetime_path: Option, }, - SelfDocs { - #[arg( - long, - default_value_t = false, - long_help = "Only check for changes, do not generate the docs" - )] - check: bool, - }, - /// Verify that any non-root global.json files are symlinks to the root global.json. GlobalJsonPolicy, /// Checks that publishable crates satisfy publish constraints. @@ -370,6 +363,24 @@ enum CiCmd { VersionUpgradeCheck, /// Builds the docs site. Docs, + /// Workflows that are not part of ci.yml should live here. + OtherWorkflows { + #[command(subcommand)] + cmd: OtherWorkflowsCmd, + }, +} + +#[derive(Subcommand)] +enum OtherWorkflowsCmd { + /// Generates this cargo ci README and checks for changes. + SelfDocs { + #[arg( + long, + default_value_t = false, + long_help = "Only check for changes, do not generate the docs" + )] + check: bool, + }, /// Interacts with CLA Assistant. ClaAssistant { #[command(subcommand)] @@ -384,6 +395,10 @@ fn run_all_clap_subcommands(skips: &[String]) -> Result<()> { .collect::>(); for subcmd in subcmds { + if subcmd == OTHER_WORKFLOWS_SUBCOMMAND { + log::info!("skipping {subcmd} because it is not part of ci.yml"); + continue; + } if skips.contains(&subcmd) { log::info!("skipping {subcmd} as requested"); continue; @@ -768,14 +783,16 @@ fn main() -> Result<()> { } } - Some(CiCmd::SelfDocs { check }) => { + Some(CiCmd::OtherWorkflows { + cmd: OtherWorkflowsCmd::SelfDocs { check }, + }) => { let readme_content = ci_docs::generate_cli_docs(); let path = Path::new(README_PATH); if check { let existing = fs::read_to_string(path).unwrap_or_default(); if existing != readme_content { - bail!("README.md is out of date. Please run `cargo ci self-docs` to update it."); + bail!("README.md is out of date. Please run `cargo ci other-workflows self-docs` to update it."); } else { log::info!("README.md is up to date."); } @@ -805,7 +822,9 @@ fn main() -> Result<()> { run_docs_build()?; } - Some(CiCmd::ClaAssistant { cmd }) => { + Some(CiCmd::OtherWorkflows { + cmd: OtherWorkflowsCmd::ClaAssistant { cmd }, + }) => { cla_assistant::run(cmd)?; } From 5705cf4310b3963cf0df59c35d86be235c258849 Mon Sep 17 00:00:00 2001 From: Zeke Foppa <196249+bfops@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:38:47 -0700 Subject: [PATCH 08/33] Apply suggestion from @bfops Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> --- tools/ci/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index b0dee4ccebb..7d64c520a89 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -363,7 +363,7 @@ enum CiCmd { VersionUpgradeCheck, /// Builds the docs site. Docs, - /// Workflows that are not part of ci.yml should live here. + /// Workflows that are not part of ci.yml should live here. They will not be run as part of a no-subcommand invocation of `cargo ci`. OtherWorkflows { #[command(subcommand)] cmd: OtherWorkflowsCmd, From feeaddb4a447d45c74a8721f042b8a5934323975 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:39:45 -0400 Subject: [PATCH 09/33] Use default skip for other workflows --- tools/ci/src/main.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 7d64c520a89..48d87bbe113 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -57,7 +57,7 @@ struct Cli { /// When no subcommand is specified, all subcommands are run in sequence. This option allows /// specifying subcommands to skip when running all. For example, to skip the `unreal-tests` /// subcommand, use `--skip unreal-tests`. - #[arg(long)] + #[arg(long, default_value = OTHER_WORKFLOWS_SUBCOMMAND)] skip: Vec, } @@ -395,10 +395,6 @@ fn run_all_clap_subcommands(skips: &[String]) -> Result<()> { .collect::>(); for subcmd in subcmds { - if subcmd == OTHER_WORKFLOWS_SUBCOMMAND { - log::info!("skipping {subcmd} because it is not part of ci.yml"); - continue; - } if skips.contains(&subcmd) { log::info!("skipping {subcmd} as requested"); continue; From 3564ccafa6ac6448fd54f23bd7d3aedc3f5978fe Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:46:20 -0400 Subject: [PATCH 10/33] Inline other-workflows skip default --- tools/ci/src/main.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 48d87bbe113..010c6531221 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -19,8 +19,6 @@ mod util; use util::ensure_repo_root; -const OTHER_WORKFLOWS_SUBCOMMAND: &str = "other-workflows"; - /// On Windows, `pnpm` is installed as a `.cmd` shim which `CreateProcess` cannot /// find without going through the shell. Wrapping with `cmd /c` fixes this. /// On Unix, we invoke `pnpm` directly. @@ -57,7 +55,7 @@ struct Cli { /// When no subcommand is specified, all subcommands are run in sequence. This option allows /// specifying subcommands to skip when running all. For example, to skip the `unreal-tests` /// subcommand, use `--skip unreal-tests`. - #[arg(long, default_value = OTHER_WORKFLOWS_SUBCOMMAND)] + #[arg(long, default_value = "other-workflows")] skip: Vec, } From 62a6ba577a30d57d68a7b37d1f3abb6b642b63f2 Mon Sep 17 00:00:00 2001 From: Zeke Foppa <196249+bfops@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:47:12 -0700 Subject: [PATCH 11/33] Apply suggestion from @bfops Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> --- tools/ci/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 010c6531221..7aea7c02ea3 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -361,7 +361,7 @@ enum CiCmd { VersionUpgradeCheck, /// Builds the docs site. Docs, - /// Workflows that are not part of ci.yml should live here. They will not be run as part of a no-subcommand invocation of `cargo ci`. + /// Workflows should leave here if they should not be run as part of a no-subcommand invocation of `cargo ci`. OtherWorkflows { #[command(subcommand)] cmd: OtherWorkflowsCmd, From ecb12015d004a4d9572498de7acd49c27166acd9 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:49:22 -0400 Subject: [PATCH 12/33] Move self-docs back to top level --- .github/workflows/ci.yml | 2 +- tools/ci/README.md | 16 ++++++++-------- tools/ci/src/ci_docs.rs | 2 +- tools/ci/src/main.rs | 24 +++++++++++------------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4fb638cf6b7..38ba84da17f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -728,7 +728,7 @@ jobs: run: rustup default $(rustup show active-toolchain | cut -d' ' -f1) - name: Check for docs change - run: cargo ci other-workflows self-docs --check + run: cargo ci self-docs --check cli_docs: needs: [merge_queue_noop] diff --git a/tools/ci/README.md b/tools/ci/README.md index 98b3b1d627b..8e62ff405f8 100644 --- a/tools/ci/README.md +++ b/tools/ci/README.md @@ -248,28 +248,28 @@ Usage: docs - `--help`: Print help -### `other-workflows` +### `self-docs` **Usage:** ```bash -Usage: other-workflows +Usage: self-docs [OPTIONS] ``` **Options:** -- `--help`: Print help +- `--check`: Only check for changes, do not generate the docs +- `--help`: Print help (see a summary with '-h') -#### `self-docs` +### `other-workflows` **Usage:** ```bash -Usage: self-docs [OPTIONS] +Usage: other-workflows ``` **Options:** -- `--check`: Only check for changes, do not generate the docs -- `--help`: Print help (see a summary with '-h') +- `--help`: Print help #### `cla-assistant` @@ -348,5 +348,5 @@ Usage: help [COMMAND]... This document is auto-generated by running: ```bash -cargo ci other-workflows self-docs +cargo ci self-docs ``` \ No newline at end of file diff --git a/tools/ci/src/ci_docs.rs b/tools/ci/src/ci_docs.rs index 8be5f79136c..3fadcdaf0cc 100644 --- a/tools/ci/src/ci_docs.rs +++ b/tools/ci/src/ci_docs.rs @@ -21,7 +21,7 @@ This document provides an overview of the `cargo ci` command-line tool, and docu This document is auto-generated by running: ```bash -cargo ci other-workflows self-docs +cargo ci self-docs ```", usage ) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 7aea7c02ea3..b434a3c62f0 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -361,15 +361,6 @@ enum CiCmd { VersionUpgradeCheck, /// Builds the docs site. Docs, - /// Workflows should leave here if they should not be run as part of a no-subcommand invocation of `cargo ci`. - OtherWorkflows { - #[command(subcommand)] - cmd: OtherWorkflowsCmd, - }, -} - -#[derive(Subcommand)] -enum OtherWorkflowsCmd { /// Generates this cargo ci README and checks for changes. SelfDocs { #[arg( @@ -379,6 +370,15 @@ enum OtherWorkflowsCmd { )] check: bool, }, + /// Workflows should leave here if they should not be run as part of a no-subcommand invocation of `cargo ci`. + OtherWorkflows { + #[command(subcommand)] + cmd: OtherWorkflowsCmd, + }, +} + +#[derive(Subcommand)] +enum OtherWorkflowsCmd { /// Interacts with CLA Assistant. ClaAssistant { #[command(subcommand)] @@ -777,16 +777,14 @@ fn main() -> Result<()> { } } - Some(CiCmd::OtherWorkflows { - cmd: OtherWorkflowsCmd::SelfDocs { check }, - }) => { + Some(CiCmd::SelfDocs { check }) => { let readme_content = ci_docs::generate_cli_docs(); let path = Path::new(README_PATH); if check { let existing = fs::read_to_string(path).unwrap_or_default(); if existing != readme_content { - bail!("README.md is out of date. Please run `cargo ci other-workflows self-docs` to update it."); + bail!("README.md is out of date. Please run `cargo ci self-docs` to update it."); } else { log::info!("README.md is up to date."); } From f05295d95d59f2f10db23e31ca68982de499c107 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 14:54:51 -0400 Subject: [PATCH 13/33] Restore self-docs command position --- tools/ci/README.md | 24 ++++++++++++------------ tools/ci/src/main.rs | 19 ++++++++++--------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/tools/ci/README.md b/tools/ci/README.md index 8e62ff405f8..855041eeeef 100644 --- a/tools/ci/README.md +++ b/tools/ci/README.md @@ -193,6 +193,18 @@ Usage: cli-docs [OPTIONS] - `--spacetime-path `: specify a custom path to the SpacetimeDB repository root (where the main Cargo.toml is located) - `--help`: Print help (see a summary with '-h') +### `self-docs` + +**Usage:** +```bash +Usage: self-docs [OPTIONS] +``` + +**Options:** + +- `--check`: Only check for changes, do not generate the docs +- `--help`: Print help (see a summary with '-h') + ### `global-json-policy` **Usage:** @@ -248,18 +260,6 @@ Usage: docs - `--help`: Print help -### `self-docs` - -**Usage:** -```bash -Usage: self-docs [OPTIONS] -``` - -**Options:** - -- `--check`: Only check for changes, do not generate the docs -- `--help`: Print help (see a summary with '-h') - ### `other-workflows` **Usage:** diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index b434a3c62f0..61ff40f481d 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -351,6 +351,16 @@ enum CiCmd { )] spacetime_path: Option, }, + /// Generates this cargo ci README and checks for changes. + SelfDocs { + #[arg( + long, + default_value_t = false, + long_help = "Only check for changes, do not generate the docs" + )] + check: bool, + }, + /// Verify that any non-root global.json files are symlinks to the root global.json. GlobalJsonPolicy, /// Checks that publishable crates satisfy publish constraints. @@ -361,15 +371,6 @@ enum CiCmd { VersionUpgradeCheck, /// Builds the docs site. Docs, - /// Generates this cargo ci README and checks for changes. - SelfDocs { - #[arg( - long, - default_value_t = false, - long_help = "Only check for changes, do not generate the docs" - )] - check: bool, - }, /// Workflows should leave here if they should not be run as part of a no-subcommand invocation of `cargo ci`. OtherWorkflows { #[command(subcommand)] From 94c52ad95ac7cfb595cc72c38422c90933a1bcdc Mon Sep 17 00:00:00 2001 From: Zeke Foppa <196249+bfops@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:55:57 -0700 Subject: [PATCH 14/33] Apply suggestion from @bfops Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> --- tools/ci/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 61ff40f481d..cf30cffa7c3 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -351,7 +351,6 @@ enum CiCmd { )] spacetime_path: Option, }, - /// Generates this cargo ci README and checks for changes. SelfDocs { #[arg( long, From 96d0cde48bdd932f61112bc9e0ffcef98a4a271f Mon Sep 17 00:00:00 2001 From: Zeke Foppa <196249+bfops@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:59:14 -0700 Subject: [PATCH 15/33] Apply suggestion from @bfops Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> --- tools/ci/src/codeowners_check.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index d45bd357773..cdca8519b22 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -19,7 +19,6 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { } } - println!("CODEOWNERS-controlled file changes are allowed."); Ok(()) } From b8359a745406c44b7e784b621495eb0ac98cad21 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 15:07:56 -0400 Subject: [PATCH 16/33] Make CODEOWNERS review gate explicit --- tools/ci/src/codeowners_check.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index cdca8519b22..d401861ad72 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -15,7 +15,9 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { for path in changed_files(base_ref)? { if is_license_file(&path) { - require_review_from(base_ref, &path, pr_number, REQUIRED_LICENSE_REVIEWER)?; + if !is_trivial_license_change(base_ref, &path)? { + require_review_from(&path, pr_number, REQUIRED_LICENSE_REVIEWER)?; + } } } @@ -46,7 +48,7 @@ fn changed_files(base_ref: &str) -> Result> { Ok(output.lines().map(Path::new).map(Path::to_path_buf).collect()) } -fn require_review_from(base_ref: &str, path: &Path, pr_number: u64, reviewer: &str) -> Result<()> { +fn is_trivial_license_change(base_ref: &str, path: &Path) -> Result { let diff = cmd!( "git", "diff", @@ -58,10 +60,10 @@ fn require_review_from(base_ref: &str, path: &Path, pr_number: u64, reviewer: &s ) .read() .with_context(|| format!("failed to read diff for {}", path.display()))?; - if diff_only_changes_license_version_or_date(&diff) { - return Ok(()); - } + Ok(diff_only_changes_license_version_or_date(&diff)) +} +fn require_review_from(path: &Path, pr_number: u64, reviewer: &str) -> Result<()> { if approved_by(pr_number, reviewer)? { return Ok(()); } From d93165511019b529de754a8053e53e8481f65b67 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 15:10:28 -0400 Subject: [PATCH 17/33] Cache CODEOWNERS review status lookup --- tools/ci/src/codeowners_check.rs | 50 ++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index d401861ad72..92d65e5d572 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -2,6 +2,7 @@ use anyhow::{anyhow, bail, Context, Result}; use duct::cmd; use regex::Regex; use serde_json::Value; +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::OnceLock; @@ -13,10 +14,11 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { fetch_base_ref(base_ref)?; + let mut review_statuses = ReviewStatuses::new(pr_number); for path in changed_files(base_ref)? { if is_license_file(&path) { if !is_trivial_license_change(base_ref, &path)? { - require_review_from(&path, pr_number, REQUIRED_LICENSE_REVIEWER)?; + require_review_from(&path, &mut review_statuses, REQUIRED_LICENSE_REVIEWER)?; } } } @@ -63,15 +65,42 @@ fn is_trivial_license_change(base_ref: &str, path: &Path) -> Result { Ok(diff_only_changes_license_version_or_date(&diff)) } -fn require_review_from(path: &Path, pr_number: u64, reviewer: &str) -> Result<()> { - if approved_by(pr_number, reviewer)? { +struct ReviewStatuses { + pr_number: u64, + latest_by_author: Option>, +} + +impl ReviewStatuses { + fn new(pr_number: u64) -> Self { + Self { + pr_number, + latest_by_author: None, + } + } + + fn approved_by(&mut self, reviewer: &str) -> Result { + if self.latest_by_author.is_none() { + self.latest_by_author = Some(latest_review_status_by_author(self.pr_number)?); + } + + Ok(self + .latest_by_author + .as_ref() + .and_then(|statuses| statuses.get(reviewer)) + .is_some_and(|state| state == "APPROVED")) + } +} + +fn require_review_from(path: &Path, review_statuses: &mut ReviewStatuses, reviewer: &str) -> Result<()> { + if review_statuses.approved_by(reviewer)? { return Ok(()); } bail!( - "{} has changes beyond version numbers and change dates, and PR #{pr_number} \ + "{} has changes beyond version numbers and change dates, and PR #{} \ does not have an approval from {reviewer}", - path.display() + path.display(), + review_statuses.pr_number ); } @@ -150,7 +179,7 @@ fn normalized_change_date_line(line: &str) -> String { .into_owned() } -fn approved_by(pr_number: u64, reviewer: &str) -> Result { +fn latest_review_status_by_author(pr_number: u64) -> Result> { let reviews_json = cmd!( "gh", "api", @@ -163,21 +192,18 @@ fn approved_by(pr_number: u64, reviewer: &str) -> Result { .as_array() .ok_or_else(|| anyhow!("GitHub reviews response was not an array"))?; - let mut latest_approval = false; + let mut latest_by_author = HashMap::new(); for review in reviews { let Some(login) = review.pointer("/user/login").and_then(Value::as_str) else { continue; }; - if login != reviewer { - continue; - } let Some(state) = review.get("state").and_then(Value::as_str) else { continue; }; - latest_approval = state == "APPROVED"; + latest_by_author.insert(login.to_string(), state.to_string()); } - Ok(latest_approval) + Ok(latest_by_author) } #[cfg(test)] From 422dbf134841c79e22419b9822319e7557213b5c Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 15:12:54 -0400 Subject: [PATCH 18/33] Move license policy into module --- tools/ci/src/codeowners_check.rs | 254 ++++++++++++++++--------------- 1 file changed, 132 insertions(+), 122 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index 92d65e5d572..e67fb908924 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -1,13 +1,10 @@ use anyhow::{anyhow, bail, Context, Result}; use duct::cmd; -use regex::Regex; use serde_json::Value; use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; const REPO: &str = "clockworklabs/SpacetimeDB"; -const REQUIRED_LICENSE_REVIEWER: &str = "cloutiertyler"; pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { super::ensure_repo_root()?; @@ -16,11 +13,7 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { let mut review_statuses = ReviewStatuses::new(pr_number); for path in changed_files(base_ref)? { - if is_license_file(&path) { - if !is_trivial_license_change(base_ref, &path)? { - require_review_from(&path, &mut review_statuses, REQUIRED_LICENSE_REVIEWER)?; - } - } + license::check(base_ref, &path, &mut review_statuses)?; } Ok(()) @@ -50,21 +43,6 @@ fn changed_files(base_ref: &str) -> Result> { Ok(output.lines().map(Path::new).map(Path::to_path_buf).collect()) } -fn is_trivial_license_change(base_ref: &str, path: &Path) -> Result { - let diff = cmd!( - "git", - "diff", - "--unified=0", - "--no-ext-diff", - &format!("{base_ref}...HEAD"), - "--", - path - ) - .read() - .with_context(|| format!("failed to read diff for {}", path.display()))?; - Ok(diff_only_changes_license_version_or_date(&diff)) -} - struct ReviewStatuses { pr_number: u64, latest_by_author: Option>, @@ -104,81 +82,6 @@ fn require_review_from(path: &Path, review_statuses: &mut ReviewStatuses, review ); } -fn is_license_file(path: &Path) -> bool { - let path_str = path.to_string_lossy(); - if path_str.starts_with("licenses/") { - return true; - } - - path.file_name() - .and_then(|name| name.to_str()) - .is_some_and(|name| name.to_ascii_lowercase().starts_with("license")) -} - -fn diff_only_changes_license_version_or_date(diff: &str) -> bool { - let mut pending_removal: Option<&str> = None; - let mut saw_change = false; - - for line in diff.lines() { - if line.starts_with("diff --git ") - || line.starts_with("index ") - || line.starts_with("--- ") - || line.starts_with("+++ ") - || line.starts_with("@@") - { - continue; - } - - if let Some(removed) = line.strip_prefix('-') { - if pending_removal.replace(removed).is_some() { - return false; - } - saw_change = true; - continue; - } - - if let Some(added) = line.strip_prefix('+') { - let Some(removed) = pending_removal.take() else { - return false; - }; - if !allowed_license_line_change(removed, added) { - return false; - } - saw_change = true; - continue; - } - - if pending_removal.is_some() { - return false; - } - } - - pending_removal.is_none() && saw_change -} - -fn allowed_license_line_change(old: &str, new: &str) -> bool { - normalized_license_version_line(old) == normalized_license_version_line(new) - && normalized_license_version_line(old) != old - || normalized_change_date_line(old) == normalized_change_date_line(new) - && normalized_change_date_line(old) != old -} - -fn normalized_license_version_line(line: &str) -> String { - static VERSION: OnceLock = OnceLock::new(); - VERSION - .get_or_init(|| Regex::new(r"\bSpacetimeDB \d+\.\d+\.\d+\b").unwrap()) - .replace_all(line, "SpacetimeDB ") - .into_owned() -} - -fn normalized_change_date_line(line: &str) -> String { - static CHANGE_DATE: OnceLock = OnceLock::new(); - CHANGE_DATE - .get_or_init(|| Regex::new(r"\bChange Date:\s+\d{4}-\d{2}-\d{2}\b").unwrap()) - .replace_all(line, "Change Date: ") - .into_owned() -} - fn latest_review_status_by_author(pr_number: u64) -> Result> { let reviews_json = cmd!( "gh", @@ -206,13 +109,119 @@ fn latest_review_status_by_author(pr_number: u64) -> Result bool { + let path_str = path.to_string_lossy(); + if path_str.starts_with("licenses/") { + return true; + } + + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.to_ascii_lowercase().starts_with("license")) + } + + fn diff_only_changes_version_or_date(diff: &str) -> bool { + let mut pending_removal: Option<&str> = None; + let mut saw_change = false; + + for line in diff.lines() { + if line.starts_with("diff --git ") + || line.starts_with("index ") + || line.starts_with("--- ") + || line.starts_with("+++ ") + || line.starts_with("@@") + { + continue; + } + + if let Some(removed) = line.strip_prefix('-') { + if pending_removal.replace(removed).is_some() { + return false; + } + saw_change = true; + continue; + } + + if let Some(added) = line.strip_prefix('+') { + let Some(removed) = pending_removal.take() else { + return false; + }; + if !allowed_line_change(removed, added) { + return false; + } + saw_change = true; + continue; + } + + if pending_removal.is_some() { + return false; + } + } + + pending_removal.is_none() && saw_change + } + + fn allowed_line_change(old: &str, new: &str) -> bool { + normalized_version_line(old) == normalized_version_line(new) && normalized_version_line(old) != old + || normalized_change_date_line(old) == normalized_change_date_line(new) + && normalized_change_date_line(old) != old + } + + fn normalized_version_line(line: &str) -> String { + static VERSION: OnceLock = OnceLock::new(); + VERSION + .get_or_init(|| Regex::new(r"\bSpacetimeDB \d+\.\d+\.\d+\b").unwrap()) + .replace_all(line, "SpacetimeDB ") + .into_owned() + } + + fn normalized_change_date_line(line: &str) -> String { + static CHANGE_DATE: OnceLock = OnceLock::new(); + CHANGE_DATE + .get_or_init(|| Regex::new(r"\bChange Date:\s+\d{4}-\d{2}-\d{2}\b").unwrap()) + .replace_all(line, "Change Date: ") + .into_owned() + } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn allows_version_and_change_date_updates() { + let diff = "\ diff --git a/LICENSE.txt b/LICENSE.txt index 111..222 100644 --- a/LICENSE.txt @@ -225,12 +234,12 @@ index 111..222 100644 +Change Date: 2031-06-01 "; - assert!(diff_only_changes_license_version_or_date(diff)); - } + assert!(diff_only_changes_version_or_date(diff)); + } - #[test] - fn rejects_unpaired_license_addition() { - let diff = "\ + #[test] + fn rejects_unpaired_license_addition() { + let diff = "\ diff --git a/LICENSE.txt b/LICENSE.txt --- a/LICENSE.txt +++ b/LICENSE.txt @@ -238,12 +247,12 @@ diff --git a/LICENSE.txt b/LICENSE.txt +New license term "; - assert!(!diff_only_changes_license_version_or_date(diff)); - } + assert!(!diff_only_changes_version_or_date(diff)); + } - #[test] - fn rejects_non_version_license_edit() { - let diff = "\ + #[test] + fn rejects_non_version_license_edit() { + let diff = "\ diff --git a/LICENSE.txt b/LICENSE.txt --- a/LICENSE.txt +++ b/LICENSE.txt @@ -252,14 +261,15 @@ diff --git a/LICENSE.txt b/LICENSE.txt +New license term "; - assert!(!diff_only_changes_license_version_or_date(diff)); - } + assert!(!diff_only_changes_version_or_date(diff)); + } - #[test] - fn detects_license_files() { - assert!(is_license_file(Path::new("LICENSE.txt"))); - assert!(is_license_file(Path::new("crates/cli/LICENSE"))); - assert!(is_license_file(Path::new("licenses/BSL.txt"))); - assert!(!is_license_file(Path::new("crates/cli/Cargo.toml"))); + #[test] + fn detects_license_files() { + assert!(is_license_file(Path::new("LICENSE.txt"))); + assert!(is_license_file(Path::new("crates/cli/LICENSE"))); + assert!(is_license_file(Path::new("licenses/BSL.txt"))); + assert!(!is_license_file(Path::new("crates/cli/Cargo.toml"))); + } } } From 588eae3f79a9d35ca7327db4742cf199c52aa530 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 15:22:57 -0400 Subject: [PATCH 19/33] Use global CODEOWNERS review state --- tools/ci/src/codeowners_check.rs | 116 +++++++++++++++++-------------- 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index e67fb908924..4327f409c77 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -3,22 +3,33 @@ use duct::cmd; use serde_json::Value; use std::collections::HashMap; use std::path::{Path, PathBuf}; +use std::sync::OnceLock; const REPO: &str = "clockworklabs/SpacetimeDB"; +static REVIEW_STATUSES: OnceLock = OnceLock::new(); pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { super::ensure_repo_root()?; fetch_base_ref(base_ref)?; + initialize_review_statuses(pr_number)?; - let mut review_statuses = ReviewStatuses::new(pr_number); for path in changed_files(base_ref)? { - license::check(base_ref, &path, &mut review_statuses)?; + if license::is_file(&path) && !license::is_trivial_change(base_ref, &path)? { + require_review_from(&path, "cloutiertyler")?; + } } Ok(()) } +fn initialize_review_statuses(pr_number: u64) -> Result<()> { + if REVIEW_STATUSES.set(ReviewStatuses::new(pr_number)).is_err() { + bail!("CODEOWNERS review status cache was already initialized"); + } + Ok(()) +} + fn fetch_base_ref(base_ref: &str) -> Result<()> { let Some(ref_name) = base_ref.strip_prefix("origin/") else { return Ok(()); @@ -45,88 +56,85 @@ fn changed_files(base_ref: &str) -> Result> { struct ReviewStatuses { pr_number: u64, - latest_by_author: Option>, + latest_by_author: OnceLock>, } impl ReviewStatuses { fn new(pr_number: u64) -> Self { Self { pr_number, - latest_by_author: None, + latest_by_author: OnceLock::new(), } } - fn approved_by(&mut self, reviewer: &str) -> Result { - if self.latest_by_author.is_none() { - self.latest_by_author = Some(latest_review_status_by_author(self.pr_number)?); + fn approved_by(&self, reviewer: &str) -> Result { + if self.latest_by_author.get().is_none() { + let latest_by_author = self.latest_review_status_by_author()?; + let _ = self.latest_by_author.set(latest_by_author); } Ok(self .latest_by_author - .as_ref() + .get() .and_then(|statuses| statuses.get(reviewer)) .is_some_and(|state| state == "APPROVED")) } + + fn latest_review_status_by_author(&self) -> Result> { + let reviews_json = cmd!( + "gh", + "api", + &format!("repos/{REPO}/pulls/{}/reviews?per_page=100", self.pr_number) + ) + .read() + .with_context(|| format!("failed to read reviews for PR #{}", self.pr_number))?; + let reviews: Value = serde_json::from_str(&reviews_json)?; + let reviews = reviews + .as_array() + .ok_or_else(|| anyhow!("GitHub reviews response was not an array"))?; + + let mut latest_by_author = HashMap::new(); + for review in reviews { + let Some(login) = review.pointer("/user/login").and_then(Value::as_str) else { + continue; + }; + let Some(state) = review.get("state").and_then(Value::as_str) else { + continue; + }; + latest_by_author.insert(login.to_string(), state.to_string()); + } + + Ok(latest_by_author) + } +} + +fn review_statuses() -> &'static ReviewStatuses { + REVIEW_STATUSES + .get() + .expect("CODEOWNERS review status cache was not initialized") } -fn require_review_from(path: &Path, review_statuses: &mut ReviewStatuses, reviewer: &str) -> Result<()> { +fn require_review_from(path: &Path, reviewer: &str) -> Result<()> { + let review_statuses = review_statuses(); if review_statuses.approved_by(reviewer)? { return Ok(()); } bail!( - "{} has changes beyond version numbers and change dates, and PR #{} \ - does not have an approval from {reviewer}", + "{} requires approval from {reviewer} on PR #{}", path.display(), review_statuses.pr_number ); } -fn latest_review_status_by_author(pr_number: u64) -> Result> { - let reviews_json = cmd!( - "gh", - "api", - &format!("repos/{REPO}/pulls/{pr_number}/reviews?per_page=100") - ) - .read() - .with_context(|| format!("failed to read reviews for PR #{pr_number}"))?; - let reviews: Value = serde_json::from_str(&reviews_json)?; - let reviews = reviews - .as_array() - .ok_or_else(|| anyhow!("GitHub reviews response was not an array"))?; - - let mut latest_by_author = HashMap::new(); - for review in reviews { - let Some(login) = review.pointer("/user/login").and_then(Value::as_str) else { - continue; - }; - let Some(state) = review.get("state").and_then(Value::as_str) else { - continue; - }; - latest_by_author.insert(login.to_string(), state.to_string()); - } - - Ok(latest_by_author) -} - mod license { - use super::{require_review_from, ReviewStatuses}; use anyhow::{Context, Result}; use duct::cmd; use regex::Regex; use std::path::Path; use std::sync::OnceLock; - const REQUIRED_REVIEWER: &str = "cloutiertyler"; - - pub(super) fn check(base_ref: &str, path: &Path, review_statuses: &mut ReviewStatuses) -> Result<()> { - if is_license_file(path) && !is_trivial_change(base_ref, path)? { - require_review_from(path, review_statuses, REQUIRED_REVIEWER)?; - } - Ok(()) - } - - fn is_trivial_change(base_ref: &str, path: &Path) -> Result { + pub(super) fn is_trivial_change(base_ref: &str, path: &Path) -> Result { let diff = cmd!( "git", "diff", @@ -141,7 +149,7 @@ mod license { Ok(diff_only_changes_version_or_date(&diff)) } - fn is_license_file(path: &Path) -> bool { + pub(super) fn is_file(path: &Path) -> bool { let path_str = path.to_string_lossy(); if path_str.starts_with("licenses/") { return true; @@ -266,10 +274,10 @@ diff --git a/LICENSE.txt b/LICENSE.txt #[test] fn detects_license_files() { - assert!(is_license_file(Path::new("LICENSE.txt"))); - assert!(is_license_file(Path::new("crates/cli/LICENSE"))); - assert!(is_license_file(Path::new("licenses/BSL.txt"))); - assert!(!is_license_file(Path::new("crates/cli/Cargo.toml"))); + assert!(is_file(Path::new("LICENSE.txt"))); + assert!(is_file(Path::new("crates/cli/LICENSE"))); + assert!(is_file(Path::new("licenses/BSL.txt"))); + assert!(!is_file(Path::new("crates/cli/Cargo.toml"))); } } } From 08346c6d6529f687f52002942e836dfafc72378b Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 15:28:46 -0400 Subject: [PATCH 20/33] Use local CODEOWNERS review state --- tools/ci/src/codeowners_check.rs | 76 +++++++++++--------------------- 1 file changed, 26 insertions(+), 50 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index 4327f409c77..effa6f33a10 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -3,30 +3,30 @@ use duct::cmd; use serde_json::Value; use std::collections::HashMap; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; const REPO: &str = "clockworklabs/SpacetimeDB"; -static REVIEW_STATUSES: OnceLock = OnceLock::new(); pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { super::ensure_repo_root()?; fetch_base_ref(base_ref)?; - initialize_review_statuses(pr_number)?; + let review = ReviewStatuses::fetch(pr_number)?; for path in changed_files(base_ref)? { - if license::is_file(&path) && !license::is_trivial_change(base_ref, &path)? { - require_review_from(&path, "cloutiertyler")?; - } + file_review_requirements(base_ref, &path, &review) + .with_context(|| format!("failed to check review requirements for {}", path.display()))?; } Ok(()) } -fn initialize_review_statuses(pr_number: u64) -> Result<()> { - if REVIEW_STATUSES.set(ReviewStatuses::new(pr_number)).is_err() { - bail!("CODEOWNERS review status cache was already initialized"); +fn file_review_requirements(base_ref: &str, path: &Path, review: &ReviewStatuses) -> Result<()> { + if license::is_file(path) { + if !license::is_trivial_change(base_ref, path)? { + review.require("cloutiertyler")?; + } } + Ok(()) } @@ -56,38 +56,18 @@ fn changed_files(base_ref: &str) -> Result> { struct ReviewStatuses { pr_number: u64, - latest_by_author: OnceLock>, + latest_by_author: HashMap, } impl ReviewStatuses { - fn new(pr_number: u64) -> Self { - Self { - pr_number, - latest_by_author: OnceLock::new(), - } - } - - fn approved_by(&self, reviewer: &str) -> Result { - if self.latest_by_author.get().is_none() { - let latest_by_author = self.latest_review_status_by_author()?; - let _ = self.latest_by_author.set(latest_by_author); - } - - Ok(self - .latest_by_author - .get() - .and_then(|statuses| statuses.get(reviewer)) - .is_some_and(|state| state == "APPROVED")) - } - - fn latest_review_status_by_author(&self) -> Result> { + fn fetch(pr_number: u64) -> Result { let reviews_json = cmd!( "gh", "api", - &format!("repos/{REPO}/pulls/{}/reviews?per_page=100", self.pr_number) + &format!("repos/{REPO}/pulls/{pr_number}/reviews?per_page=100") ) .read() - .with_context(|| format!("failed to read reviews for PR #{}", self.pr_number))?; + .with_context(|| format!("failed to read reviews for PR #{pr_number}"))?; let reviews: Value = serde_json::from_str(&reviews_json)?; let reviews = reviews .as_array() @@ -104,27 +84,23 @@ impl ReviewStatuses { latest_by_author.insert(login.to_string(), state.to_string()); } - Ok(latest_by_author) + Ok(Self { + pr_number, + latest_by_author, + }) } -} -fn review_statuses() -> &'static ReviewStatuses { - REVIEW_STATUSES - .get() - .expect("CODEOWNERS review status cache was not initialized") -} + fn require(&self, reviewer: &str) -> Result<()> { + if self + .latest_by_author + .get(reviewer) + .is_some_and(|state| state == "APPROVED") + { + return Ok(()); + } -fn require_review_from(path: &Path, reviewer: &str) -> Result<()> { - let review_statuses = review_statuses(); - if review_statuses.approved_by(reviewer)? { - return Ok(()); + bail!("PR #{} does not have approval from {reviewer}", self.pr_number); } - - bail!( - "{} requires approval from {reviewer} on PR #{}", - path.display(), - review_statuses.pr_number - ); } mod license { From 6053fad297d0b149de1721cff1b514c0a7176e19 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 15:29:41 -0400 Subject: [PATCH 21/33] Rename CODEOWNERS review status --- tools/ci/src/codeowners_check.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index effa6f33a10..2ebe9995ebd 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -10,7 +10,7 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { super::ensure_repo_root()?; fetch_base_ref(base_ref)?; - let review = ReviewStatuses::fetch(pr_number)?; + let review = ReviewStatus::fetch(pr_number)?; for path in changed_files(base_ref)? { file_review_requirements(base_ref, &path, &review) @@ -20,7 +20,7 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { Ok(()) } -fn file_review_requirements(base_ref: &str, path: &Path, review: &ReviewStatuses) -> Result<()> { +fn file_review_requirements(base_ref: &str, path: &Path, review: &ReviewStatus) -> Result<()> { if license::is_file(path) { if !license::is_trivial_change(base_ref, path)? { review.require("cloutiertyler")?; @@ -54,12 +54,12 @@ fn changed_files(base_ref: &str) -> Result> { Ok(output.lines().map(Path::new).map(Path::to_path_buf).collect()) } -struct ReviewStatuses { +struct ReviewStatus { pr_number: u64, latest_by_author: HashMap, } -impl ReviewStatuses { +impl ReviewStatus { fn fetch(pr_number: u64) -> Result { let reviews_json = cmd!( "gh", From 12c85eeec70fc7743d5e653641f71a9a4e37368d Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 15:30:20 -0400 Subject: [PATCH 22/33] Clarify CODEOWNERS review error context --- tools/ci/src/codeowners_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index 2ebe9995ebd..d1edb6b5ca5 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -14,7 +14,7 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { for path in changed_files(base_ref)? { file_review_requirements(base_ref, &path, &review) - .with_context(|| format!("failed to check review requirements for {}", path.display()))?; + .with_context(|| format!("review requirements failed for {}", path.display()))?; } Ok(()) From b819e7d08d2214693b96de8d918b806ea442b090 Mon Sep 17 00:00:00 2001 From: Zeke Foppa <196249+bfops@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:31:20 -0700 Subject: [PATCH 23/33] Apply suggestion from @bfops Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> --- tools/ci/src/codeowners_check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index d1edb6b5ca5..5bec9e9a103 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -99,7 +99,7 @@ impl ReviewStatus { return Ok(()); } - bail!("PR #{} does not have approval from {reviewer}", self.pr_number); + bail!("needs approval from {reviewer}"); } } From f3332b41e464c7135a77bde251b00333751e88a2 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 4 Aug 2026 12:33:28 -0700 Subject: [PATCH 24/33] [bot/codeowners-license-check]: update CODEOWNERS --- .github/CODEOWNERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 578fd6da031..7ed75409a55 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2,12 +2,14 @@ /rust-toolchain.toml @cloutiertyler LICENSE @cloutiertyler LICENSE.txt @cloutiertyler +/LICENSE.txt @cloutiertyler @jdetter /licenses/ @cloutiertyler /crates/client-api-messages/src/websocket.rs @centril @gefjon /crates/cli/src/ @bfops @cloutiertyler @jdetter /tools/ci/ @bfops @cloutiertyler @jdetter /tools/ci/src/keynote_bench.rs @joshua-spacetime @cloutiertyler @jdetter +/tools/ci/src/codeowners_check.rs @cloutiertyler /tools/upgrade-version/ @bfops @jdetter @cloutiertyler @rekhoff /tools/release/ @bfops @jdetter @cloutiertyler @rekhoff /tools/license-check/ @bfops @jdetter @cloutiertyler From 4bbd76d69890d0d34bd01d4bc8790bdd65d9576a Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 16:45:21 -0400 Subject: [PATCH 25/33] Fix CODEOWNERS lint failures --- .github/workflows/ci.yml | 2 ++ tools/ci/src/codeowners_check.rs | 12 +++--------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 499b0983e10..815ea378a01 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -516,6 +516,8 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - uses: dsherret/rust-toolchain-file@v1 - name: Set default rust toolchain diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index 5bec9e9a103..9a8fc581fb8 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -21,10 +21,8 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { } fn file_review_requirements(base_ref: &str, path: &Path, review: &ReviewStatus) -> Result<()> { - if license::is_file(path) { - if !license::is_trivial_change(base_ref, path)? { - review.require("cloutiertyler")?; - } + if license::is_file(path) && !license::is_trivial_change(base_ref, path)? { + review.require("cloutiertyler")?; } Ok(()) @@ -55,7 +53,6 @@ fn changed_files(base_ref: &str) -> Result> { } struct ReviewStatus { - pr_number: u64, latest_by_author: HashMap, } @@ -84,10 +81,7 @@ impl ReviewStatus { latest_by_author.insert(login.to_string(), state.to_string()); } - Ok(Self { - pr_number, - latest_by_author, - }) + Ok(Self { latest_by_author }) } fn require(&self, reviewer: &str) -> Result<()> { From 08e8ab6438a80ca3696a8a1d2e6cc62d545203fc Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Tue, 4 Aug 2026 16:49:49 -0400 Subject: [PATCH 26/33] Rename CODEOWNERS license predicate --- tools/ci/src/codeowners_check.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index 9a8fc581fb8..006d597cf87 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -21,7 +21,7 @@ pub fn run(base_ref: &str, pr_number: u64) -> Result<()> { } fn file_review_requirements(base_ref: &str, path: &Path, review: &ReviewStatus) -> Result<()> { - if license::is_file(path) && !license::is_trivial_change(base_ref, path)? { + if license::is_license(path) && !license::is_trivial_change(base_ref, path)? { review.require("cloutiertyler")?; } @@ -119,7 +119,7 @@ mod license { Ok(diff_only_changes_version_or_date(&diff)) } - pub(super) fn is_file(path: &Path) -> bool { + pub(super) fn is_license(path: &Path) -> bool { let path_str = path.to_string_lossy(); if path_str.starts_with("licenses/") { return true; @@ -244,10 +244,10 @@ diff --git a/LICENSE.txt b/LICENSE.txt #[test] fn detects_license_files() { - assert!(is_file(Path::new("LICENSE.txt"))); - assert!(is_file(Path::new("crates/cli/LICENSE"))); - assert!(is_file(Path::new("licenses/BSL.txt"))); - assert!(!is_file(Path::new("crates/cli/Cargo.toml"))); + assert!(is_license(Path::new("LICENSE.txt"))); + assert!(is_license(Path::new("crates/cli/LICENSE"))); + assert!(is_license(Path::new("licenses/BSL.txt"))); + assert!(!is_license(Path::new("crates/cli/Cargo.toml"))); } } } From 047e6a6ac91411d2666d241d5956e3ab79384171 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Wed, 5 Aug 2026 16:21:40 -0400 Subject: [PATCH 27/33] Make CODEOWNERS check pass in merge queue --- .github/workflows/ci.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 815ea378a01..d4246efc3a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -504,7 +504,7 @@ jobs: retention-days: 30 codeowners_check: - if: ${{ github.event_name == 'pull_request' }} + if: ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }} name: CODEOWNERS check runs-on: ubuntu-latest permissions: @@ -515,15 +515,25 @@ jobs: RUST_BACKTRACE: full GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: + - name: Pass redundant merge queue check + if: ${{ github.event_name == 'merge_group' }} + run: | + echo "CODEOWNERS check already ran on the pull request before it entered the merge queue." + echo "This merge queue check is redundant and intentionally passes." + - uses: actions/checkout@v4 + if: ${{ github.event_name == 'pull_request' }} with: fetch-depth: 0 - uses: dsherret/rust-toolchain-file@v1 + if: ${{ github.event_name == 'pull_request' }} - name: Set default rust toolchain + if: ${{ github.event_name == 'pull_request' }} run: rustup default $(rustup show active-toolchain | cut -d' ' -f1) - name: Cache Rust dependencies + if: ${{ github.event_name == 'pull_request' }} uses: Swatinem/rust-cache@v2 with: workspaces: ${{ github.workspace }} @@ -532,6 +542,7 @@ jobs: prefix-key: v1 - name: Run CODEOWNERS check + if: ${{ github.event_name == 'pull_request' }} run: | cargo ci other-workflows codeowners-check \ --base-ref "origin/${{ github.event.pull_request.base.ref }}" \ From 116bae3f92cd7ed7feb26fa91e9ec09d5a9e64e2 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Wed, 5 Aug 2026 16:25:16 -0400 Subject: [PATCH 28/33] Split CODEOWNERS merge queue check --- .github/workflows/ci.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d4246efc3a6..440900c2530 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -504,7 +504,7 @@ jobs: retention-days: 30 codeowners_check: - if: ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }} + if: ${{ github.event_name == 'pull_request' }} name: CODEOWNERS check runs-on: ubuntu-latest permissions: @@ -515,25 +515,15 @@ jobs: RUST_BACKTRACE: full GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - - name: Pass redundant merge queue check - if: ${{ github.event_name == 'merge_group' }} - run: | - echo "CODEOWNERS check already ran on the pull request before it entered the merge queue." - echo "This merge queue check is redundant and intentionally passes." - - uses: actions/checkout@v4 - if: ${{ github.event_name == 'pull_request' }} with: fetch-depth: 0 - uses: dsherret/rust-toolchain-file@v1 - if: ${{ github.event_name == 'pull_request' }} - name: Set default rust toolchain - if: ${{ github.event_name == 'pull_request' }} run: rustup default $(rustup show active-toolchain | cut -d' ' -f1) - name: Cache Rust dependencies - if: ${{ github.event_name == 'pull_request' }} uses: Swatinem/rust-cache@v2 with: workspaces: ${{ github.workspace }} @@ -542,12 +532,22 @@ jobs: prefix-key: v1 - name: Run CODEOWNERS check - if: ${{ github.event_name == 'pull_request' }} run: | cargo ci other-workflows codeowners-check \ --base-ref "origin/${{ github.event.pull_request.base.ref }}" \ --pr-number "${{ github.event.pull_request.number }}" + codeowners_check_merge_group: + if: ${{ github.event_name == 'merge_group' }} + name: CODEOWNERS check + runs-on: ubuntu-latest + permissions: read-all + steps: + - name: Pass redundant merge queue check + run: | + echo "CODEOWNERS check already ran on the pull request before it entered the merge queue." + echo "This merge queue check is redundant and intentionally passes." + wasm_bindings: needs: [merge_queue_noop] if: ${{ needs.merge_queue_noop.outputs.skip != 'true' }} From d21fe4a7863899970a23162cf3a14b050e601710 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Wed, 5 Aug 2026 13:29:06 -0700 Subject: [PATCH 29/33] [bot/codeowners-license-check]: review --- .github/workflows/ci.yml | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 440900c2530..86d15f83e05 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -506,7 +506,7 @@ jobs: codeowners_check: if: ${{ github.event_name == 'pull_request' }} name: CODEOWNERS check - runs-on: ubuntu-latest + runs-on: spacetimedb-new-runner-2 permissions: contents: read pull-requests: read @@ -537,17 +537,6 @@ jobs: --base-ref "origin/${{ github.event.pull_request.base.ref }}" \ --pr-number "${{ github.event.pull_request.number }}" - codeowners_check_merge_group: - if: ${{ github.event_name == 'merge_group' }} - name: CODEOWNERS check - runs-on: ubuntu-latest - permissions: read-all - steps: - - name: Pass redundant merge queue check - run: | - echo "CODEOWNERS check already ran on the pull request before it entered the merge queue." - echo "This merge queue check is redundant and intentionally passes." - wasm_bindings: needs: [merge_queue_noop] if: ${{ needs.merge_queue_noop.outputs.skip != 'true' }} From 2ab814259b09edef861b31d6571de068b827d6e9 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Sat, 8 Aug 2026 15:43:39 -0400 Subject: [PATCH 30/33] Add John to license CODEOWNERS --- .github/CODEOWNERS | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 7ed75409a55..b59befee035 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,9 +1,9 @@ /crates/core/src/db/datastore/traits.rs @cloutiertyler /rust-toolchain.toml @cloutiertyler -LICENSE @cloutiertyler -LICENSE.txt @cloutiertyler +LICENSE @cloutiertyler @jdetter +LICENSE.txt @cloutiertyler @jdetter /LICENSE.txt @cloutiertyler @jdetter -/licenses/ @cloutiertyler +/licenses/ @cloutiertyler @jdetter /crates/client-api-messages/src/websocket.rs @centril @gefjon /crates/cli/src/ @bfops @cloutiertyler @jdetter From 42e75bae0d1d4396d1ac70a924c7ed7aa2353699 Mon Sep 17 00:00:00 2001 From: Zeke Foppa <196249+bfops@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:54:33 -0700 Subject: [PATCH 31/33] Apply suggestion from @bfops Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> --- .github/CODEOWNERS | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index b59befee035..fe44d81d136 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2,7 +2,6 @@ /rust-toolchain.toml @cloutiertyler LICENSE @cloutiertyler @jdetter LICENSE.txt @cloutiertyler @jdetter -/LICENSE.txt @cloutiertyler @jdetter /licenses/ @cloutiertyler @jdetter /crates/client-api-messages/src/websocket.rs @centril @gefjon From 4d33de52c3bc6aad3b9b2358e5fa7b5cfa17fcc6 Mon Sep 17 00:00:00 2001 From: clockwork-labs-bot Date: Sat, 8 Aug 2026 15:55:17 -0400 Subject: [PATCH 32/33] Test whole-file license diffs --- tools/ci/src/codeowners_check.rs | 34 ++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tools/ci/src/codeowners_check.rs b/tools/ci/src/codeowners_check.rs index 006d597cf87..0457806d8f7 100644 --- a/tools/ci/src/codeowners_check.rs +++ b/tools/ci/src/codeowners_check.rs @@ -228,6 +228,40 @@ diff --git a/LICENSE.txt b/LICENSE.txt assert!(!diff_only_changes_version_or_date(diff)); } + #[test] + fn rejects_entirely_added_license_file() { + let diff = "\ +diff --git a/licenses/new.txt b/licenses/new.txt +new file mode 100644 +index 0000000..1111111 +--- /dev/null ++++ b/licenses/new.txt +@@ -0,0 +1,3 @@ ++Licensed Work: SpacetimeDB 2.4.0 ++Change Date: 2031-06-01 ++New license term +"; + + assert!(!diff_only_changes_version_or_date(diff)); + } + + #[test] + fn rejects_entirely_removed_license_file() { + let diff = "\ +diff --git a/licenses/old.txt b/licenses/old.txt +deleted file mode 100644 +index 1111111..0000000 +--- a/licenses/old.txt ++++ /dev/null +@@ -1,3 +0,0 @@ +-Licensed Work: SpacetimeDB 2.3.0 +-Change Date: 2031-05-26 +-Old license term +"; + + assert!(!diff_only_changes_version_or_date(diff)); + } + #[test] fn rejects_non_version_license_edit() { let diff = "\ From 30a951271a83afa72d64c0c78f51ed17403a702f Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Sat, 8 Aug 2026 12:59:10 -0700 Subject: [PATCH 33/33] [bot/codeowners-bsl-license]: comments --- .github/CODEOWNERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fe44d81d136..0aeaa6338a8 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,8 +1,11 @@ /crates/core/src/db/datastore/traits.rs @cloutiertyler /rust-toolchain.toml @cloutiertyler + +# These license files are all covered by the `CODEOWNERS check` CI check as well LICENSE @cloutiertyler @jdetter LICENSE.txt @cloutiertyler @jdetter /licenses/ @cloutiertyler @jdetter + /crates/client-api-messages/src/websocket.rs @centril @gefjon /crates/cli/src/ @bfops @cloutiertyler @jdetter