From 96cefa84cffc9c6d386180acd45a4ff4b922e72b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 12 Apr 2026 13:35:57 -0400 Subject: [PATCH] refactor(async): lint std::process::Command across all targets Move async subprocess paths to Tokio or spawn_blocking, document the intentional synchronous std::process::Command callsites, and make CI run Clippy with --all-targets so the guardrail applies to test code too. --- .github/workflows/rust.yml | 2 +- clippy.toml | 1 + lib/crates/fabro-checkpoint/src/metadata.rs | 5 + lib/crates/fabro-cli/build.rs | 4 + lib/crates/fabro-cli/src/commands/doctor.rs | 43 +++--- lib/crates/fabro-cli/src/commands/install.rs | 121 +++++++++------- lib/crates/fabro-cli/src/commands/pr/close.rs | 2 +- .../fabro-cli/src/commands/pr/create.rs | 2 +- lib/crates/fabro-cli/src/commands/pr/list.rs | 2 +- lib/crates/fabro-cli/src/commands/pr/merge.rs | 2 +- lib/crates/fabro-cli/src/commands/pr/mod.rs | 3 +- lib/crates/fabro-cli/src/commands/pr/view.rs | 2 +- .../fabro-cli/src/commands/repo/init.rs | 12 +- .../fabro-cli/src/commands/run/preview.rs | 6 +- .../fabro-cli/src/commands/run/runner.rs | 7 +- lib/crates/fabro-cli/src/commands/run/ssh.rs | 4 + .../fabro-cli/src/commands/server/record.rs | 4 + .../fabro-cli/src/commands/server/start.rs | 12 +- lib/crates/fabro-cli/src/commands/upgrade.rs | 3 +- lib/crates/fabro-cli/src/shared/github.rs | 3 +- .../fabro-cli/src/sleep_inhibitor/linux.rs | 8 ++ .../fabro-cli/tests/it/cmd/json_global.rs | 5 + lib/crates/fabro-cli/tests/it/cmd/runner.rs | 9 +- lib/crates/fabro-cli/tests/it/cmd/support.rs | 4 + .../fabro-cli/tests/it/scenario/recovery.rs | 5 + .../fabro-config/tests/resolve_server.rs | 4 +- lib/crates/fabro-github/src/lib.rs | 6 +- lib/crates/fabro-graphviz/src/render.rs | 8 ++ lib/crates/fabro-sandbox/src/local.rs | 57 ++++++-- lib/crates/fabro-server/src/diagnostics.rs | 15 +- lib/crates/fabro-server/src/jwt_auth.rs | 5 + lib/crates/fabro-server/src/run_manifest.rs | 4 +- lib/crates/fabro-telemetry/src/spawn.rs | 4 + lib/crates/fabro-template/src/lib.rs | 3 +- lib/crates/fabro-test/src/lib.rs | 12 ++ lib/crates/fabro-workflow/src/git.rs | 12 ++ .../fabro-workflow/src/pipeline/initialize.rs | 31 ++-- lib/crates/fabro-workflow/src/sandbox_git.rs | 5 + .../src/transforms/file_inlining.rs | 5 + .../tests/it/daytona_integration.rs | 4 + .../tests/it/git_integration.rs | 135 +++++++++--------- .../fabro-workflow/tests/it/integration.rs | 4 + test/twin/github/src/handlers/git.rs | 4 + test/twin/github/src/state.rs | 12 ++ test/twin/openai/tests/debug_ui.rs | 5 + 45 files changed, 406 insertions(+), 200 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 0bb72a6b8..0e1a55e37 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -53,7 +53,7 @@ jobs: - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2 with: cache-on-failure: true - - run: cargo clippy --workspace -- -D warnings + - run: cargo clippy --workspace --all-targets -- -D warnings test: name: Test (Linux) diff --git a/clippy.toml b/clippy.toml index 5680497c0..3a1eb764c 100644 --- a/clippy.toml +++ b/clippy.toml @@ -4,4 +4,5 @@ disallowed-methods = [ { path = "std::thread::sleep", reason = "Prefer tokio::time::sleep on Tokio paths; document intentional blocking sleeps with #[expect(clippy::disallowed_methods, reason = \"...\")]", replacement = "tokio::time::sleep" }, { path = "std::thread::spawn", reason = "Prefer Tokio task APIs on async paths; document intentional dedicated OS threads with #[expect(clippy::disallowed_methods, reason = \"...\")]" }, { path = "std::thread::Builder::spawn", reason = "Prefer Tokio task APIs on async paths; document intentional dedicated OS threads with #[expect(clippy::disallowed_methods, reason = \"...\")]" }, + { path = "std::process::Command::new", reason = "Prefer tokio::process::Command on Tokio paths; document intentional synchronous subprocesses with #[expect(clippy::disallowed_methods, reason = \"...\")]" }, ] diff --git a/lib/crates/fabro-checkpoint/src/metadata.rs b/lib/crates/fabro-checkpoint/src/metadata.rs index d0afc7d15..a25d6f41f 100644 --- a/lib/crates/fabro-checkpoint/src/metadata.rs +++ b/lib/crates/fabro-checkpoint/src/metadata.rs @@ -179,6 +179,11 @@ impl MetadataStore { #[cfg(test)] mod tests { + #![expect( + clippy::disallowed_methods, + reason = "These unit tests use the real git CLI to validate metadata branch behavior." + )] + use std::collections::HashMap; use chrono::{TimeZone, Utc}; diff --git a/lib/crates/fabro-cli/build.rs b/lib/crates/fabro-cli/build.rs index 6ec1c3c48..c24a4d15a 100644 --- a/lib/crates/fabro-cli/build.rs +++ b/lib/crates/fabro-cli/build.rs @@ -1,3 +1,7 @@ +#[expect( + clippy::disallowed_methods, + reason = "Build scripts run outside Tokio and need a synchronous git probe for the embedded build SHA." +)] fn main() { println!("cargo:rerun-if-changed=../../../.git/HEAD"); diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index 02d3fa387..57fe554f8 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -1,5 +1,4 @@ use std::path::PathBuf; -use std::process::Command; use std::sync::LazyLock; use anyhow::Result; @@ -17,6 +16,7 @@ use fabro_util::terminal::Styles; use fabro_util::version::FABRO_VERSION; use regex::Regex; use semver::Version; +use tokio::process::Command as TokioCommand; use crate::args::{DoctorArgs, GlobalArgs}; use crate::command_context::CommandContext; @@ -68,28 +68,29 @@ fn parse_version(re: &Regex, output: &str) -> Option { )) } -pub(crate) fn probe_system_deps() -> Vec { - DEP_SPECS - .iter() - .map(|spec| { - let result = Command::new(spec.command[0]) - .args(&spec.command[1..]) - .output() - .ok(); +pub(crate) async fn probe_system_deps() -> Vec { + let mut outcomes = Vec::with_capacity(DEP_SPECS.len()); + for spec in DEP_SPECS { + let result = TokioCommand::new(spec.command[0]) + .args(&spec.command[1..]) + .output() + .await + .ok(); - match result { - None => ProbeOutcome::NotFound, - Some(output) if !output.status.success() => ProbeOutcome::Failed, - Some(output) => { - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - let version = parse_version(spec.pattern, &stdout) - .or_else(|| parse_version(spec.pattern, &stderr)); - ProbeOutcome::Ok { version } - } + let outcome = match result { + None => ProbeOutcome::NotFound, + Some(output) if !output.status.success() => ProbeOutcome::Failed, + Some(output) => { + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let version = parse_version(spec.pattern, &stdout) + .or_else(|| parse_version(spec.pattern, &stderr)); + ProbeOutcome::Ok { version } } - }) - .collect() + }; + outcomes.push(outcome); + } + outcomes } fn dep_issue(name: &str, issue: &str, required: bool) -> (CheckStatus, String) { diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 2ccbfcd78..01873bf32 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -1,7 +1,6 @@ -use std::io::Write as _; use std::net::SocketAddr; use std::path::Path; -use std::process::{Command, Stdio}; +use std::process::Stdio; use anyhow::{Context, Result, anyhow, bail}; use axum::extract::Query; @@ -20,7 +19,9 @@ use fabro_server::secret_store::SecretStore; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use rand::Rng; +use tokio::io::AsyncWriteExt; use tokio::net::TcpListener; +use tokio::process::Command as TokioCommand; use tokio::sync::oneshot; use tokio::task::spawn_blocking; @@ -38,10 +39,11 @@ use crate::{server_client, user_config}; // --------------------------------------------------------------------------- /// Run an openssl subcommand and return stdout on success. -fn run_openssl(args: &[&str], description: &str) -> Result> { - let output = Command::new("openssl") +async fn run_openssl(args: &[&str], description: &str) -> Result> { + let output = TokioCommand::new("openssl") .args(args) .output() + .await .with_context(|| format!("failed to run openssl for: {description}"))?; if !output.status.success() { bail!( @@ -53,22 +55,30 @@ fn run_openssl(args: &[&str], description: &str) -> Result> { } /// Run an openssl subcommand that reads key material from stdin. -fn run_openssl_with_stdin(args: &[&str], stdin_data: &[u8], description: &str) -> Result> { - let mut child = Command::new("openssl") +async fn run_openssl_with_stdin( + args: &[&str], + stdin_data: &[u8], + description: &str, +) -> Result> { + let mut child = TokioCommand::new("openssl") .args(args) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() .with_context(|| format!("failed to spawn openssl for: {description}"))?; - child + let mut stdin = child .stdin .take() - .context("openssl process missing stdin")? + .context("openssl process missing stdin")?; + stdin .write_all(stdin_data) + .await .with_context(|| format!("failed to write to openssl stdin for: {description}"))?; + drop(stdin); let output = child .wait_with_output() + .await .with_context(|| format!("failed to read openssl output for: {description}"))?; if !output.status.success() { bail!( @@ -93,10 +103,11 @@ fn generate_session_secret() -> String { // JWT keypair generation // --------------------------------------------------------------------------- -fn generate_jwt_keypair() -> Result<(String, String)> { - let private_pem = run_openssl(&["genpkey", "-algorithm", "Ed25519"], "generate keypair")?; +async fn generate_jwt_keypair() -> Result<(String, String)> { + let private_pem = + run_openssl(&["genpkey", "-algorithm", "Ed25519"], "generate keypair").await?; let public_pem = - run_openssl_with_stdin(&["pkey", "-pubout"], &private_pem, "extract public key")?; + run_openssl_with_stdin(&["pkey", "-pubout"], &private_pem, "extract public key").await?; let private_str = String::from_utf8(private_pem).context("private key is not valid UTF-8")?; let public_str = String::from_utf8(public_pem).context("public key is not valid UTF-8")?; @@ -107,11 +118,11 @@ fn generate_jwt_keypair() -> Result<(String, String)> { // mTLS certificate generation // --------------------------------------------------------------------------- -fn generate_mtls_certs(dir: &Path) -> Result<()> { +async fn generate_mtls_certs(dir: &Path) -> Result<()> { std::fs::create_dir_all(dir).context("failed to create certs directory")?; // 1. CA key + self-signed cert - let ca_key = run_openssl(&["genpkey", "-algorithm", "Ed25519"], "generate CA key")?; + let ca_key = run_openssl(&["genpkey", "-algorithm", "Ed25519"], "generate CA key").await?; let ca_key_path = dir.join("ca.key"); std::fs::write(&ca_key_path, &ca_key)?; @@ -130,12 +141,14 @@ fn generate_mtls_certs(dir: &Path) -> Result<()> { "/CN=Fabro CA", ], "generate CA cert", - )?; + ) + .await?; let ca_cert_path = dir.join("ca.crt"); std::fs::write(&ca_cert_path, &ca_cert)?; // 2. Server key + CSR signed by CA - let server_key = run_openssl(&["genpkey", "-algorithm", "Ed25519"], "generate server key")?; + let server_key = + run_openssl(&["genpkey", "-algorithm", "Ed25519"], "generate server key").await?; let server_key_path = dir.join("server.key"); std::fs::write(&server_key_path, &server_key)?; @@ -151,7 +164,8 @@ fn generate_mtls_certs(dir: &Path) -> Result<()> { "/CN=localhost", ], "generate server CSR", - )?; + ) + .await?; let csr_path = dir.join("server.csr"); std::fs::write(&csr_path, &csr)?; @@ -175,7 +189,8 @@ fn generate_mtls_certs(dir: &Path) -> Result<()> { "3650", ], "sign server cert", - )?; + ) + .await?; std::fs::write(dir.join("server.crt"), &server_cert)?; // Clean up temporary files @@ -316,12 +331,13 @@ fn format_config_toml(username: &str) -> String { // --------------------------------------------------------------------------- /// Check if a binary exists on PATH using the doctor.rs pattern. -fn detect_binary_on_path(binary: &str) -> bool { - Command::new(binary) +async fn detect_binary_on_path(binary: &str) -> bool { + TokioCommand::new(binary) .arg("--version") .stdout(Stdio::null()) .stderr(Stdio::null()) .status() + .await .map(|s| s.success()) .unwrap_or(false) } @@ -756,7 +772,7 @@ pub(crate) async fn run_install( " {}", s.dim.apply_to("[Pre-flight] System dependency checks") ); - let dep_outcomes = doctor::probe_system_deps(); + let dep_outcomes = doctor::probe_system_deps().await; let dep_check = doctor::check_system_deps(doctor::DEP_SPECS, &dep_outcomes); if dep_check.status == doctor::CheckStatus::Error { @@ -777,9 +793,10 @@ pub(crate) async fn run_install( .await??; if install { - let status = Command::new("brew") + let status = TokioCommand::new("brew") .args(["install", "graphviz"]) .status() + .await .context("failed to run brew install graphviz")?; if !status.success() { fabro_util::printerr!(printer, " Warning: brew install graphviz failed"); @@ -802,7 +819,7 @@ pub(crate) async fn run_install( let mut secret_pairs: Vec<(String, String)> = Vec::new(); let mut configured_providers: Vec = Vec::new(); - let codex_detected = detect_binary_on_path("codex"); + let codex_detected = detect_binary_on_path("codex").await; let mut openai_via_oauth = false; if codex_detected { @@ -897,7 +914,7 @@ pub(crate) async fn run_install( match strategy { 0 => { - let token = fabro_github::gh_auth_token().map_err(|err| { + let token = fabro_github::gh_auth_token().await.map_err(|err| { anyhow!("{err}. Run `gh auth login` and rerun `fabro install`.") })?; let user_toml_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); @@ -1007,7 +1024,7 @@ pub(crate) async fn run_install( s.green.apply_to("✔") ); - let (jwt_private_pem, jwt_public_pem) = generate_jwt_keypair()?; + let (jwt_private_pem, jwt_public_pem) = generate_jwt_keypair().await?; fabro_util::printerr!( printer, " {} Ed25519 JWT keypair generated", @@ -1015,7 +1032,7 @@ pub(crate) async fn run_install( ); let certs_dir = fabro_dir.join("certs"); - generate_mtls_certs(&certs_dir)?; + generate_mtls_certs(&certs_dir).await?; fabro_util::printerr!( printer, " {} mTLS CA + server certificates generated", @@ -1105,14 +1122,14 @@ mod tests { // -- Binary detection -- - #[test] - fn detect_binary_finds_existing_command() { - assert!(detect_binary_on_path("git")); + #[tokio::test] + async fn detect_binary_finds_existing_command() { + assert!(detect_binary_on_path("git").await); } - #[test] - fn detect_binary_returns_false_for_nonexistent() { - assert!(!detect_binary_on_path("arc_nonexistent_xyz")); + #[tokio::test] + async fn detect_binary_returns_false_for_nonexistent() { + assert!(!detect_binary_on_path("arc_nonexistent_xyz").await); } // -- Session secret -- @@ -1137,37 +1154,37 @@ mod tests { // -- JWT keypair -- - #[test] - fn jwt_keypair_private_pem_header() { - let (private, _) = generate_jwt_keypair().unwrap(); + #[tokio::test] + async fn jwt_keypair_private_pem_header() { + let (private, _) = generate_jwt_keypair().await.unwrap(); assert!( private.starts_with("-----BEGIN PRIVATE KEY-----"), "private PEM: {private}" ); } - #[test] - fn jwt_keypair_public_pem_header() { - let (_, public) = generate_jwt_keypair().unwrap(); + #[tokio::test] + async fn jwt_keypair_public_pem_header() { + let (_, public) = generate_jwt_keypair().await.unwrap(); assert!( public.starts_with("-----BEGIN PUBLIC KEY-----"), "public PEM: {public}" ); } - #[test] - fn jwt_keypair_public_parses() { - let (_, public) = generate_jwt_keypair().unwrap(); + #[tokio::test] + async fn jwt_keypair_public_parses() { + let (_, public) = generate_jwt_keypair().await.unwrap(); jsonwebtoken::DecodingKey::from_ed_pem(public.as_bytes()).expect("public key should parse"); } // -- mTLS cert generation -- - #[test] - fn mtls_certs_creates_files() { + #[tokio::test] + async fn mtls_certs_creates_files() { let dir = tempfile::tempdir().unwrap(); let certs_dir = dir.path().join("certs"); - generate_mtls_certs(&certs_dir).unwrap(); + generate_mtls_certs(&certs_dir).await.unwrap(); assert!(certs_dir.join("ca.key").exists()); assert!(certs_dir.join("ca.crt").exists()); @@ -1175,11 +1192,11 @@ mod tests { assert!(certs_dir.join("server.crt").exists()); } - #[test] - fn mtls_ca_cert_is_pem() { + #[tokio::test] + async fn mtls_ca_cert_is_pem() { let dir = tempfile::tempdir().unwrap(); let certs_dir = dir.path().join("certs"); - generate_mtls_certs(&certs_dir).unwrap(); + generate_mtls_certs(&certs_dir).await.unwrap(); let ca_crt = std::fs::read_to_string(certs_dir.join("ca.crt")).unwrap(); assert!( @@ -1188,11 +1205,11 @@ mod tests { ); } - #[test] - fn mtls_server_cert_is_pem() { + #[tokio::test] + async fn mtls_server_cert_is_pem() { let dir = tempfile::tempdir().unwrap(); let certs_dir = dir.path().join("certs"); - generate_mtls_certs(&certs_dir).unwrap(); + generate_mtls_certs(&certs_dir).await.unwrap(); let server_crt = std::fs::read_to_string(certs_dir.join("server.crt")).unwrap(); assert!( @@ -1201,11 +1218,11 @@ mod tests { ); } - #[test] - fn mtls_certs_parse_via_rustls() { + #[tokio::test] + async fn mtls_certs_parse_via_rustls() { let dir = tempfile::tempdir().unwrap(); let certs_dir = dir.path().join("certs"); - generate_mtls_certs(&certs_dir).unwrap(); + generate_mtls_certs(&certs_dir).await.unwrap(); let ca_pem = std::fs::read(certs_dir.join("ca.crt")).unwrap(); let mut reader = std::io::Cursor::new(&ca_pem); diff --git a/lib/crates/fabro-cli/src/commands/pr/close.rs b/lib/crates/fabro-cli/src/commands/pr/close.rs index a1e63aacf..8dc9523f8 100644 --- a/lib/crates/fabro-cli/src/commands/pr/close.rs +++ b/lib/crates/fabro-cli/src/commands/pr/close.rs @@ -12,7 +12,7 @@ pub(super) async fn close_command( ) -> Result<()> { let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, printer).await?; - let creds = super::load_github_credentials_required(printer)?; + let creds = super::load_github_credentials_required(printer).await?; fabro_github::close_pull_request( &creds, diff --git a/lib/crates/fabro-cli/src/commands/pr/create.rs b/lib/crates/fabro-cli/src/commands/pr/create.rs index 922ce69b9..cb8ddcf2f 100644 --- a/lib/crates/fabro-cli/src/commands/pr/create.rs +++ b/lib/crates/fabro-cli/src/commands/pr/create.rs @@ -71,7 +71,7 @@ pub(super) async fn create_command( let (owner, repo) = fabro_github::parse_github_owner_repo(&https_url) .map_err(|err| anyhow::anyhow!("{err}"))?; - let creds = super::load_github_credentials_required(printer)?; + let creds = super::load_github_credentials_required(printer).await?; let branch_found = fabro_github::branch_exists( &creds, diff --git a/lib/crates/fabro-cli/src/commands/pr/list.rs b/lib/crates/fabro-cli/src/commands/pr/list.rs index 051703887..53dae4201 100644 --- a/lib/crates/fabro-cli/src/commands/pr/list.rs +++ b/lib/crates/fabro-cli/src/commands/pr/list.rs @@ -44,7 +44,7 @@ pub(super) async fn list_command( return Ok(()); } - let creds = super::load_github_credentials_required(printer)?; + let creds = super::load_github_credentials_required(printer).await?; let futures: Vec<_> = entries .iter() diff --git a/lib/crates/fabro-cli/src/commands/pr/merge.rs b/lib/crates/fabro-cli/src/commands/pr/merge.rs index 34ac28d3b..4e8f35f4f 100644 --- a/lib/crates/fabro-cli/src/commands/pr/merge.rs +++ b/lib/crates/fabro-cli/src/commands/pr/merge.rs @@ -12,7 +12,7 @@ pub(super) async fn merge_command( ) -> Result<()> { let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, printer).await?; - let creds = super::load_github_credentials_required(printer)?; + let creds = super::load_github_credentials_required(printer).await?; fabro_github::merge_pull_request( &creds, diff --git a/lib/crates/fabro-cli/src/commands/pr/mod.rs b/lib/crates/fabro-cli/src/commands/pr/mod.rs index 6af52a6d8..1a2a7b355 100644 --- a/lib/crates/fabro-cli/src/commands/pr/mod.rs +++ b/lib/crates/fabro-cli/src/commands/pr/mod.rs @@ -31,7 +31,7 @@ pub(crate) async fn dispatch( } } -fn load_github_credentials_required(printer: Printer) -> Result { +async fn load_github_credentials_required(printer: Printer) -> Result { let ctx = CommandContext::base(printer)?; let server_settings = fabro_config::resolve_server_from_file(ctx.machine_settings()).map_err(|errors| { @@ -54,6 +54,7 @@ fn load_github_credentials_required(printer: Printer) -> Result Result<()> { let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, printer).await?; - let creds = super::load_github_credentials_required(printer)?; + let creds = super::load_github_credentials_required(printer).await?; let detail = fabro_github::get_pull_request( &creds, diff --git a/lib/crates/fabro-cli/src/commands/repo/init.rs b/lib/crates/fabro-cli/src/commands/repo/init.rs index 085f58d69..204144c87 100644 --- a/lib/crates/fabro-cli/src/commands/repo/init.rs +++ b/lib/crates/fabro-cli/src/commands/repo/init.rs @@ -2,11 +2,16 @@ use std::path::PathBuf; use anyhow::{Context, Result, bail}; use fabro_util::printer::Printer; +use tokio::process::Command as TokioCommand; use tokio::task::spawn_blocking; use crate::args::{GlobalArgs, RepoInitArgs, ServerTargetArgs}; use crate::command_context::CommandContext; +#[expect( + clippy::disallowed_methods, + reason = "This is a shared synchronous git helper used by repo deinit; async callers should use spawn_blocking." +)] pub(super) fn git_repo_root() -> Result { let output = std::process::Command::new("git") .args(["rev-parse", "--show-toplevel"]) @@ -27,7 +32,9 @@ pub(crate) async fn run_init( globals: &GlobalArgs, printer: Printer, ) -> Result> { - let repo_root = git_repo_root()?; + let repo_root = spawn_blocking(git_repo_root) + .await + .context("git repo root task panicked")??; let mut created = Vec::new(); let fabro_dir = repo_root.join(".fabro"); @@ -144,9 +151,10 @@ draft = true async fn check_github_app_installation(target: &ServerTargetArgs, printer: Printer) { // Get the git remote origin URL - let output = match std::process::Command::new("git") + let output = match TokioCommand::new("git") .args(["remote", "get-url", "origin"]) .output() + .await { Ok(o) if o.status.success() => o, _ => { diff --git a/lib/crates/fabro-cli/src/commands/run/preview.rs b/lib/crates/fabro-cli/src/commands/run/preview.rs index a39b1f0dc..67da6dd0f 100644 --- a/lib/crates/fabro-cli/src/commands/run/preview.rs +++ b/lib/crates/fabro-cli/src/commands/run/preview.rs @@ -52,7 +52,11 @@ pub(crate) async fn run(args: PreviewArgs, globals: &GlobalArgs, printer: Printe } if args.open && !globals.json { - std::process::Command::new("open") + #[expect( + clippy::disallowed_methods, + reason = "Preview URL opening is a fire-and-forget OS integration, not a Tokio-managed child process." + )] + let _browser = std::process::Command::new("open") .arg(&response.url) .spawn() .context("Failed to open browser")?; diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index 1adef0fbb..6e3a01b48 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -75,7 +75,7 @@ pub(crate) async fn execute( spawn_worker_control_stream(Arc::clone(&interviewer), Arc::clone(&cancel_token))?; let run_control = RunControlState::new(); install_signal_handlers(Arc::clone(&run_control), Arc::clone(&cancel_token))?; - let github_app = maybe_build_github_credentials(&run_record.settings)?; + let github_app = maybe_build_github_credentials(&run_record.settings).await?; let services = StartServices { run_id, cancel_token: Some(Arc::clone(&cancel_token)), @@ -470,7 +470,7 @@ fn update_worker_title_from_event(event: &RunEvent) { } } -fn maybe_build_github_credentials( +async fn maybe_build_github_credentials( settings: &SettingsLayer, ) -> Result> { let resolved_run = fabro_config::resolve_run_from_file(settings).ok(); @@ -493,11 +493,12 @@ fn maybe_build_github_credentials( .map(InterpString::as_source); if required_github_credentials { - return build_github_credentials(strategy, app_id.as_deref()); + return build_github_credentials(strategy, app_id.as_deref()).await; } if pull_request_enabled { return Ok(build_github_credentials(strategy, app_id.as_deref()) + .await .ok() .flatten()); } diff --git a/lib/crates/fabro-cli/src/commands/run/ssh.rs b/lib/crates/fabro-cli/src/commands/run/ssh.rs index 9242489d5..d960f408e 100644 --- a/lib/crates/fabro-cli/src/commands/run/ssh.rs +++ b/lib/crates/fabro-cli/src/commands/run/ssh.rs @@ -44,6 +44,10 @@ fn format_output(ssh_command: &str) -> String { } #[cfg(unix)] +#[expect( + clippy::disallowed_methods, + reason = "This path replaces the current process via CommandExt::exec; Tokio child APIs are not a substitute." +)] fn exec_ssh(ssh_cmd: &str) -> Result<()> { use std::os::unix::process::CommandExt; diff --git a/lib/crates/fabro-cli/src/commands/server/record.rs b/lib/crates/fabro-cli/src/commands/server/record.rs index 27129208a..274d7c00d 100644 --- a/lib/crates/fabro-cli/src/commands/server/record.rs +++ b/lib/crates/fabro-cli/src/commands/server/record.rs @@ -79,6 +79,10 @@ pub(crate) fn active_server_record(storage_dir: &Path) -> Option { } #[cfg(unix)] +#[expect( + clippy::disallowed_methods, + reason = "This synchronous process identity probe is shared by async server start and sync server status flows." +)] fn server_process_matches(record: &ServerRecord) -> bool { let output = match std::process::Command::new("ps") .args(["-ww", "-o", "command=", "-p", &record.pid.to_string()]) diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index 2da794809..54d8d43f5 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -11,6 +11,7 @@ use fabro_server::serve; use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs}; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; +use tokio::process::Command as TokioCommand; use tokio::time; use super::record; @@ -205,7 +206,7 @@ async fn execute_daemon( let stdout_log = log_file.try_clone()?; let exe = std::env::current_exe()?; - let mut cmd = std::process::Command::new(&exe); + let mut cmd = TokioCommand::new(&exe); cmd.args(["server", "__serve"]) .arg("--record-path") .arg(&record_path) @@ -248,7 +249,7 @@ async fn execute_daemon( .stdin(std::process::Stdio::null()); #[cfg(unix)] - fabro_proc::pre_exec_setsid(&mut cmd); + fabro_proc::pre_exec_setsid(cmd.as_std_mut()); let mut child = cmd.spawn()?; @@ -269,11 +270,12 @@ async fn execute_daemon( if let Some(record) = record::read_server_record(&record_path) { if try_connect(&record.bind) { if announce { + let pid = child.id().unwrap_or_default(); maybe_warn_host_port_fallback(bind, &record.bind, printer); fabro_util::printerr!( printer, "Server started (pid {}) on {}", - child.id(), + pid, record.bind ); } @@ -295,8 +297,8 @@ async fn execute_daemon( } record::remove_server_record(&record_path); - let _ = child.kill(); - let _ = child.wait(); + let _ = child.kill().await; + let _ = child.wait().await; let tail = read_log_tail(&log_path, 20); if !tail.is_empty() { fabro_util::printerr!(printer, "{tail}"); diff --git a/lib/crates/fabro-cli/src/commands/upgrade.rs b/lib/crates/fabro-cli/src/commands/upgrade.rs index 451bc1fd0..ec0bbe8dc 100644 --- a/lib/crates/fabro-cli/src/commands/upgrade.rs +++ b/lib/crates/fabro-cli/src/commands/upgrade.rs @@ -322,7 +322,7 @@ pub(crate) async fn run_upgrade( debug!("SHA256 checksum verified"); // Extract tarball - let status = std::process::Command::new("tar") + let status = TokioCommand::new("tar") .args([ "xzf", &tarball_path.to_string_lossy(), @@ -330,6 +330,7 @@ pub(crate) async fn run_upgrade( &tmp_dir.path().to_string_lossy(), ]) .status() + .await .context("failed to run tar")?; if !status.success() { bail!("tar extraction failed"); diff --git a/lib/crates/fabro-cli/src/shared/github.rs b/lib/crates/fabro-cli/src/shared/github.rs index fc748bde6..72fa8d2c0 100644 --- a/lib/crates/fabro-cli/src/shared/github.rs +++ b/lib/crates/fabro-cli/src/shared/github.rs @@ -2,7 +2,7 @@ use anyhow::anyhow; use fabro_github::GitHubCredentials; use fabro_types::settings::server::GithubIntegrationStrategy; -pub(crate) fn build_github_credentials( +pub(crate) async fn build_github_credentials( strategy: GithubIntegrationStrategy, app_id: Option<&str>, ) -> anyhow::Result> { @@ -11,6 +11,7 @@ pub(crate) fn build_github_credentials( GitHubCredentials::from_env(app_id).map_err(|err| anyhow!(err)) } GithubIntegrationStrategy::GhCli => fabro_github::gh_auth_token() + .await .map(|token| Some(GitHubCredentials::Token(token))) .map_err(|err| anyhow!(err)), } diff --git a/lib/crates/fabro-cli/src/sleep_inhibitor/linux.rs b/lib/crates/fabro-cli/src/sleep_inhibitor/linux.rs index 94112208b..fd2cb9dac 100644 --- a/lib/crates/fabro-cli/src/sleep_inhibitor/linux.rs +++ b/lib/crates/fabro-cli/src/sleep_inhibitor/linux.rs @@ -26,6 +26,10 @@ impl LinuxSleepInhibitor { cmd.spawn() } + #[expect( + clippy::disallowed_methods, + reason = "Sleep inhibitor ownership is tied to a std::process::Child dropped synchronously with pre-exec hooks." + )] fn try_systemd_inhibit() -> Option { let mut cmd = Command::new("systemd-inhibit"); cmd.args([ @@ -52,6 +56,10 @@ impl LinuxSleepInhibitor { } } + #[expect( + clippy::disallowed_methods, + reason = "Sleep inhibitor ownership is tied to a std::process::Child dropped synchronously with pre-exec hooks." + )] fn try_gnome_inhibit() -> Option { let mut cmd = Command::new("gnome-session-inhibit"); cmd.args([ diff --git a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs index 4259f8aa1..86383ae51 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs @@ -1,3 +1,8 @@ +#![expect( + clippy::disallowed_methods, + reason = "These CLI integration tests synchronously probe for dot before exercising JSON output paths." +)] + use std::process::Command; use fabro_test::test_context; diff --git a/lib/crates/fabro-cli/tests/it/cmd/runner.rs b/lib/crates/fabro-cli/tests/it/cmd/runner.rs index 1eddedb85..53a9a497b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/runner.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/runner.rs @@ -1,3 +1,8 @@ +#![expect( + clippy::disallowed_methods, + reason = "These CLI integration tests spawn real fabro worker subprocesses and observe their lifecycle." +)] + use std::io::Read; use std::process::{Child, ExitStatus, Output, Stdio}; use std::time::{Duration, Instant}; @@ -640,11 +645,11 @@ fn worker_exits_with_retro_enabled_even_when_stdin_stays_open() { context.write_temp( ".fabro/project.toml", - r#"_version = 1 + r"_version = 1 [run.execution] retros = true -"#, +", ); context.write_temp( "retro-success.fabro", diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index 8c8965965..4d723c53d 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -3,6 +3,10 @@ clippy::manual_assert, clippy::redundant_closure_for_method_calls )] +#![expect( + clippy::disallowed_methods, + reason = "These CLI integration test helpers shell out to real git and fabro binaries while constructing fixtures." +)] use std::collections::BTreeSet; use std::path::{Path, PathBuf}; diff --git a/lib/crates/fabro-cli/tests/it/scenario/recovery.rs b/lib/crates/fabro-cli/tests/it/scenario/recovery.rs index 7d823913e..73a832aa8 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/recovery.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/recovery.rs @@ -1,3 +1,8 @@ +#![expect( + clippy::disallowed_methods, + reason = "This recovery scenario test uses the real git CLI to set up repository history for end-to-end assertions." +)] + use std::collections::BTreeSet; use std::path::Path; diff --git a/lib/crates/fabro-config/tests/resolve_server.rs b/lib/crates/fabro-config/tests/resolve_server.rs index fa4efb220..443352be8 100644 --- a/lib/crates/fabro-config/tests/resolve_server.rs +++ b/lib/crates/fabro-config/tests/resolve_server.rs @@ -165,12 +165,12 @@ strategy = "app" #[test] fn defaults_github_integration_strategy_to_gh_cli() { let file = parse( - r#" + r" _version = 1 [server.integrations.github] enabled = true -"#, +", ); let settings = diff --git a/lib/crates/fabro-github/src/lib.rs b/lib/crates/fabro-github/src/lib.rs index 286b185db..8bd61ad21 100644 --- a/lib/crates/fabro-github/src/lib.rs +++ b/lib/crates/fabro-github/src/lib.rs @@ -1,6 +1,7 @@ use base64::Engine; use base64::engine::general_purpose::STANDARD; use serde::{Deserialize, Serialize}; +use tokio::process::Command; pub const GITHUB_API_BASE_URL: &str = "https://api.github.com"; @@ -120,10 +121,11 @@ impl GitHubCredentials { } } -pub fn gh_auth_token() -> Result { - let output = std::process::Command::new("gh") +pub async fn gh_auth_token() -> Result { + let output = Command::new("gh") .args(["auth", "token"]) .output() + .await .map_err(|err| format!("Failed to run `gh auth token`: {err}"))?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); diff --git a/lib/crates/fabro-graphviz/src/render.rs b/lib/crates/fabro-graphviz/src/render.rs index 329d2c9fd..87178786d 100644 --- a/lib/crates/fabro-graphviz/src/render.rs +++ b/lib/crates/fabro-graphviz/src/render.rs @@ -89,6 +89,10 @@ pub fn postprocess_svg(raw: Vec) -> Vec { } /// Render styled DOT source into the given format via the `dot` command. +#[expect( + clippy::disallowed_methods, + reason = "This synchronous rendering helper is intentionally called behind spawn_blocking from async server code." +)] pub fn render_dot(source: &str, format: GraphFormat) -> anyhow::Result> { let styled_source = inject_dot_style_defaults(source); let mut child = match Command::new("dot") @@ -127,6 +131,10 @@ pub fn render_dot(source: &str, format: GraphFormat) -> anyhow::Result> } #[cfg(test)] +#[expect( + clippy::disallowed_methods, + reason = "This synchronous test probe checks whether dot is installed before running render assertions." +)] fn dot_is_available() -> bool { Command::new("dot") .arg("-V") diff --git a/lib/crates/fabro-sandbox/src/local.rs b/lib/crates/fabro-sandbox/src/local.rs index 209e9647b..892e258eb 100644 --- a/lib/crates/fabro-sandbox/src/local.rs +++ b/lib/crates/fabro-sandbox/src/local.rs @@ -72,6 +72,43 @@ impl LocalSandbox { self.working_directory.join(p) } } + + fn binary_on_path(binary: &str) -> bool { + let Some(paths) = std::env::var_os("PATH") else { + return false; + }; + + #[cfg(windows)] + let extensions: Vec = std::env::var_os("PATHEXT") + .map(|value| { + value + .to_string_lossy() + .split(';') + .map(|ext| ext.to_ascii_lowercase()) + .collect() + }) + .unwrap_or_else(|| vec![".exe".to_string(), ".cmd".to_string(), ".bat".to_string()]); + + for dir in std::env::split_paths(&paths) { + let candidate = dir.join(binary); + if candidate.is_file() { + return true; + } + + #[cfg(windows)] + { + if candidate.extension().is_none() { + for ext in &extensions { + if dir.join(format!("{binary}{ext}")).is_file() { + return true; + } + } + } + } + } + + false + } } #[async_trait] @@ -267,15 +304,7 @@ impl Sandbox for LocalSandbox { let full_path = self.resolve_path(path); // Try rg (ripgrep) first, fall back to grep - let use_rg = *self.rg_available.get_or_init(|| { - std::process::Command::new("rg") - .arg("--version") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .map(|s| s.success()) - .unwrap_or(false) - }); + let use_rg = *self.rg_available.get_or_init(|| Self::binary_on_path("rg")); let output = if use_rg { let mut args = vec!["-n".to_string()]; @@ -293,9 +322,10 @@ impl Sandbox for LocalSandbox { args.push(pattern.into()); args.push(full_path.to_string_lossy().into_owned()); - std::process::Command::new("rg") + Command::new("rg") .args(&args) .output() + .await .map_err(|e| format!("Failed to run rg: {e}"))? } else { let mut args = vec!["-rn".to_string()]; @@ -313,9 +343,10 @@ impl Sandbox for LocalSandbox { args.push(pattern.into()); args.push(full_path.to_string_lossy().into_owned()); - std::process::Command::new("grep") + Command::new("grep") .args(&args) .output() + .await .map_err(|e| format!("Failed to run grep: {e}"))? }; @@ -454,6 +485,10 @@ impl Sandbox for LocalSandbox { } } + #[expect( + clippy::disallowed_methods, + reason = "This synchronous host metadata probe only runs uname once while building the sandbox platform string." + )] fn os_version(&self) -> String { #[cfg(unix)] { diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index 89ea73d67..3d726d930 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -1,5 +1,4 @@ use std::path::PathBuf; -use std::process::Command; use std::sync::LazyLock; use std::time::Duration; @@ -15,6 +14,7 @@ use fabro_util::version::FABRO_VERSION; use regex::Regex; use semver::Version; use serde::Serialize; +use tokio::process::Command; use tokio::time::timeout; use crate::server::AppState; @@ -44,8 +44,8 @@ fn parse_version(re: &Regex, output: &str) -> Option { )) } -fn probe_dot() -> ProbeOutcome { - let result = Command::new("dot").arg("-V").output().ok(); +async fn probe_dot() -> ProbeOutcome { + let result = Command::new("dot").arg("-V").output().await.ok(); match result { None => ProbeOutcome::NotFound, Some(output) if !output.status.success() => ProbeOutcome::Failed, @@ -59,8 +59,8 @@ fn probe_dot() -> ProbeOutcome { } } -fn check_dot() -> CheckResult { - let outcome = probe_dot(); +async fn check_dot() -> CheckResult { + let outcome = probe_dot().await; let (status, summary, remediation) = match &outcome { ProbeOutcome::NotFound => ( CheckStatus::Warning, @@ -154,10 +154,11 @@ fn validate_session_secret(value: &str) -> Result<(), String> { } pub async fn run_all(state: &AppState) -> DiagnosticsReport { - let (llm, github, brave) = tokio::join!( + let (llm, github, brave, dot) = tokio::join!( check_llm_providers(state), check_github_app(state), check_brave_search(state), + check_dot(), ); let sandbox = check_sandbox(state); let crypto = check_crypto(state); @@ -171,7 +172,7 @@ pub async fn run_all(state: &AppState) -> DiagnosticsReport { }, CheckSection { title: "System".to_string(), - checks: vec![check_dot()], + checks: vec![dot], }, CheckSection { title: "Configuration".to_string(), diff --git a/lib/crates/fabro-server/src/jwt_auth.rs b/lib/crates/fabro-server/src/jwt_auth.rs index 2d91a6358..1a1c983ab 100644 --- a/lib/crates/fabro-server/src/jwt_auth.rs +++ b/lib/crates/fabro-server/src/jwt_auth.rs @@ -447,6 +447,11 @@ impl FromRequestParts for AuthenticatedSubject { #[cfg(test)] mod tests { + #![expect( + clippy::disallowed_methods, + reason = "These unit tests use the host openssl CLI to generate certificate fixtures for auth validation." + )] + use axum::body::{Body, to_bytes}; use axum::http::{Request, StatusCode}; use axum::response::IntoResponse; diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index 52c1b47fa..de3347cb0 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -1061,12 +1061,12 @@ app_id = "snapshotted-app-id" manifest.configs.push(types::ManifestConfig { path: Some("/tmp/project/.fabro/project.toml".to_string()), source: Some( - r#" + r" _version = 1 [run.pull_request] enabled = true -"# +" .to_string(), ), type_: types::ManifestConfigType::Project, diff --git a/lib/crates/fabro-telemetry/src/spawn.rs b/lib/crates/fabro-telemetry/src/spawn.rs index aa850b82e..cfedb0fd1 100644 --- a/lib/crates/fabro-telemetry/src/spawn.rs +++ b/lib/crates/fabro-telemetry/src/spawn.rs @@ -78,6 +78,10 @@ fn spawn_detached_unix(args: &[&str], env: &[(&str, &str)], env_remove: &[&str]) } #[cfg(windows)] +#[expect( + clippy::disallowed_methods, + reason = "Detached Windows subprocess creation requires std::process::Command creation_flags support." +)] fn spawn_detached_windows(args: &[&str], env: &[(&str, &str)], env_remove: &[&str]) { use std::os::windows::process::CommandExt; const DETACHED_PROCESS: u32 = 0x00000008; diff --git a/lib/crates/fabro-template/src/lib.rs b/lib/crates/fabro-template/src/lib.rs index 58e0d92b2..99b917430 100644 --- a/lib/crates/fabro-template/src/lib.rs +++ b/lib/crates/fabro-template/src/lib.rs @@ -146,6 +146,7 @@ mod tests { use std::collections::HashMap; use fabro_util::env::TestEnv; + use toml::map::Map; use super::*; @@ -178,7 +179,7 @@ mod tests { fn renders_nested_input_variable() { let ctx = TemplateContext::new().with_inputs(HashMap::from([( "repo".to_string(), - toml::Value::Table(toml::map::Map::from_iter([( + toml::Value::Table(Map::from_iter([( "name".to_string(), toml::Value::String("fabro".to_string()), )])), diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index 856826962..bafc20c00 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -150,6 +150,10 @@ fn session_refs() -> &'static Mutex> { SESSION_REFS.get_or_init(|| Mutex::new(HashMap::new())) } +#[expect( + clippy::disallowed_methods, + reason = "This synchronous test-support helper uses uuidgen when available to create stable unique case IDs." +)] fn test_case_id() -> String { let ulid = std::process::Command::new("uuidgen") .arg("-r") @@ -598,6 +602,10 @@ fn wait_for_server_running(server: &ServerPaths) { ); } +#[expect( + clippy::disallowed_methods, + reason = "This synchronous test-support helper launches the real fabro CLI server before reqwest clients connect to it." +)] fn ensure_server_running(fabro_bin: &Path, server: &ServerPaths, config_path: &Path) { if server_running(server) { return; @@ -1093,6 +1101,10 @@ impl TestContext { } /// Initialize a git repository in `temp_dir`. + #[expect( + clippy::disallowed_methods, + reason = "This synchronous test-support helper initializes fixture repositories with the real git CLI." + )] pub fn git_init(&self) -> &Self { std::process::Command::new("git") .args(["init"]) diff --git a/lib/crates/fabro-workflow/src/git.rs b/lib/crates/fabro-workflow/src/git.rs index e3e78f049..3d5d4ec48 100644 --- a/lib/crates/fabro-workflow/src/git.rs +++ b/lib/crates/fabro-workflow/src/git.rs @@ -27,6 +27,10 @@ fn git_error(msg: impl Into) -> Error { } /// Return a pre-configured `git` command with auto-maintenance disabled. +#[expect( + clippy::disallowed_methods, + reason = "This shared synchronous git helper layer is used by sync code; async callers must wrap it in spawn_blocking." +)] fn git_cmd(dir: &Path) -> Command { let mut cmd = Command::new("git"); cmd.args(["-c", "maintenance.auto=0", "-c", "gc.auto=0"]) @@ -323,6 +327,10 @@ mod tests { use crate::run_dump::RunDump; /// Create a temporary git repo with an initial commit. + #[expect( + clippy::disallowed_methods, + reason = "This synchronous test helper shells out to git while constructing fixture repositories." + )] fn init_repo(dir: &Path) { Command::new("git") .args(["init"]) @@ -386,6 +394,10 @@ mod tests { } #[test] + #[expect( + clippy::disallowed_methods, + reason = "This synchronous test verifies git branch listing against the real git CLI." + )] fn create_branch_and_list() { let dir = tempfile::tempdir().unwrap(); init_repo(dir.path()); diff --git a/lib/crates/fabro-workflow/src/pipeline/initialize.rs b/lib/crates/fabro-workflow/src/pipeline/initialize.rs index 0562ecf94..d07b4ac02 100644 --- a/lib/crates/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/initialize.rs @@ -85,11 +85,15 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result Result { options.run_options.display_base_sha = Some(base_sha.clone()); Ok(Some(WorktreePlan { @@ -189,9 +196,15 @@ async fn resolve_worktree_plan(options: &mut InitOptions) -> Result { - options.run_options.display_base_sha = host_repo_path - .as_ref() - .and_then(|path| git::head_sha(path).ok()); + options.run_options.display_base_sha = if let Some(path) = host_repo_path.as_ref() { + let path = path.clone(); + spawn_blocking(move || git::head_sha(&path)) + .await + .ok() + .and_then(std::result::Result::ok) + } else { + None + }; Ok(None) } WorkdirStrategy::LocalDirectory => { diff --git a/lib/crates/fabro-workflow/src/sandbox_git.rs b/lib/crates/fabro-workflow/src/sandbox_git.rs index 31b1071a9..75fff9d4e 100644 --- a/lib/crates/fabro-workflow/src/sandbox_git.rs +++ b/lib/crates/fabro-workflow/src/sandbox_git.rs @@ -238,6 +238,11 @@ pub async fn git_replace_worktree(sandbox: &dyn Sandbox, path: &str, branch: &st #[cfg(test)] mod tests { + #![expect( + clippy::disallowed_methods, + reason = "These unit tests use the real git CLI to construct sandbox-git fixture repositories." + )] + use super::*; #[tokio::test] diff --git a/lib/crates/fabro-workflow/src/transforms/file_inlining.rs b/lib/crates/fabro-workflow/src/transforms/file_inlining.rs index c63509b7a..fe1df53b2 100644 --- a/lib/crates/fabro-workflow/src/transforms/file_inlining.rs +++ b/lib/crates/fabro-workflow/src/transforms/file_inlining.rs @@ -68,6 +68,11 @@ impl Transform for FileInliningTransform { #[cfg(test)] mod tests { + #![expect( + clippy::disallowed_methods, + reason = "These unit tests use the real git CLI to build repositories for file-inlining transform coverage." + )] + use std::sync::Arc; use fabro_graphviz::graph::{AttrValue, Graph, Node}; diff --git a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs index 0ac9c3421..f316dae33 100644 --- a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs @@ -10,6 +10,10 @@ clippy::items_after_statements, clippy::print_stderr )] +#![expect( + clippy::disallowed_methods, + reason = "These Daytona integration tests use the real git CLI to prepare remote-repo fixtures for workflow runs." +)] use std::collections::HashMap; use std::collections::hash_map::DefaultHasher; diff --git a/lib/crates/fabro-workflow/tests/it/git_integration.rs b/lib/crates/fabro-workflow/tests/it/git_integration.rs index 8d8f587e2..72b508206 100644 --- a/lib/crates/fabro-workflow/tests/it/git_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/git_integration.rs @@ -1,3 +1,8 @@ +#![expect( + clippy::disallowed_methods, + reason = "These git integration tests intentionally exercise the real git CLI to validate repository helper behavior." +)] + use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; @@ -18,7 +23,7 @@ use fabro_workflow::handler::start::StartHandler; use fabro_workflow::run_options::{GitCheckpointOptions, RunOptions}; use fabro_workflow::test_support::run_graph; -fn assert_success(output: Output, context: &str) { +fn assert_success(output: &Output, context: &str) { assert!( output.status.success(), "{context} failed: {}", @@ -28,86 +33,74 @@ fn assert_success(output: Output, context: &str) { fn init_repo(dir: &Path) { std::fs::create_dir_all(dir).unwrap(); - assert_success( - Command::new("git") - .args(["init"]) - .current_dir(dir) - .output() - .unwrap(), - "git init", - ); - assert_success( - Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - "init", - ]) - .current_dir(dir) - .output() - .unwrap(), - "git commit --allow-empty", - ); + let init = Command::new("git") + .args(["init"]) + .current_dir(dir) + .output() + .unwrap(); + assert_success(&init, "git init"); + let commit = Command::new("git") + .args([ + "-c", + "user.name=test", + "-c", + "user.email=test@test", + "commit", + "--allow-empty", + "-m", + "init", + ]) + .current_dir(dir) + .output() + .unwrap(); + assert_success(&commit, "git commit --allow-empty"); } fn init_bare_remote(dir: &Path) { std::fs::create_dir_all(dir.parent().unwrap()).unwrap(); - assert_success( - Command::new("git") - .args(["init", "--bare"]) - .arg(dir) - .output() - .unwrap(), - "git init --bare", - ); + let init = Command::new("git") + .args(["init", "--bare"]) + .arg(dir) + .output() + .unwrap(); + assert_success(&init, "git init --bare"); } fn add_origin(repo_dir: &Path, remote_dir: &Path) { - assert_success( - Command::new("git") - .args(["remote", "add", "origin"]) - .arg(remote_dir) - .current_dir(repo_dir) - .output() - .unwrap(), - "git remote add origin", - ); + let output = Command::new("git") + .args(["remote", "add", "origin"]) + .arg(remote_dir) + .current_dir(repo_dir) + .output() + .unwrap(); + assert_success(&output, "git remote add origin"); } fn rename_branch(repo_dir: &Path, branch: &str) { - assert_success( - Command::new("git") - .args(["branch", "-M", branch]) - .current_dir(repo_dir) - .output() - .unwrap(), - "git branch -M", - ); + let output = Command::new("git") + .args(["branch", "-M", branch]) + .current_dir(repo_dir) + .output() + .unwrap(); + assert_success(&output, "git branch -M"); } fn empty_commit(repo_dir: &Path, message: &str) { - assert_success( - Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - message, - ]) - .current_dir(repo_dir) - .output() - .unwrap(), - "git commit --allow-empty", - ); + let output = Command::new("git") + .args([ + "-c", + "user.name=test", + "-c", + "user.email=test@test", + "commit", + "--allow-empty", + "-m", + message, + ]) + .current_dir(repo_dir) + .output() + .unwrap(); + assert_success(&output, "git commit --allow-empty"); } fn list_branch(repo_dir: &Path, branch: &str) -> String { @@ -116,7 +109,7 @@ fn list_branch(repo_dir: &Path, branch: &str) -> String { .current_dir(repo_dir) .output() .unwrap(); - assert_success(output.clone(), "git branch --list"); + assert_success(&output, "git branch --list"); String::from_utf8(output.stdout).unwrap() } @@ -293,13 +286,13 @@ async fn git_checkpoint_skips_start_node() { }); run_options.host_repo_path = Some(PathBuf::from(repo)); - run_graph( + Box::pin(run_graph( make_registry(), Arc::new(emitter), local_env(repo), &g, &run_options, - ) + )) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 0483050d2..89da5396e 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -10,6 +10,10 @@ clippy::unnecessary_literal_bound, clippy::unreadable_literal )] +#![expect( + clippy::disallowed_methods, + reason = "These end-to-end workflow integration tests use the real git CLI to verify checkpoint and branch behavior." +)] use std::collections::VecDeque; use std::collections::hash_map::DefaultHasher; diff --git a/test/twin/github/src/handlers/git.rs b/test/twin/github/src/handlers/git.rs index b77d9b82b..c45738965 100644 --- a/test/twin/github/src/handlers/git.rs +++ b/test/twin/github/src/handlers/git.rs @@ -12,6 +12,10 @@ use crate::server::SharedState; use crate::state::{PermissionLevel, TokenPermission}; /// Find the git-http-backend binary by querying `git --exec-path`. +#[expect( + clippy::disallowed_methods, + reason = "This synchronous test harness helper resolves the git-http-backend path before launching the CGI subprocess." +)] fn find_git_http_backend() -> Result { let output = std::process::Command::new("git") .arg("--exec-path") diff --git a/test/twin/github/src/state.rs b/test/twin/github/src/state.rs index 0104a8b3a..21764dd50 100644 --- a/test/twin/github/src/state.rs +++ b/test/twin/github/src/state.rs @@ -258,6 +258,10 @@ pub fn derive_public_key_pem(private_key_pem: &str) -> String { return TEST_RSA_PUBLIC_PEM.to_string(); } + #[expect( + clippy::disallowed_methods, + reason = "This synchronous test harness helper derives an RSA public key with the host openssl CLI." + )] let mut child = Command::new("openssl") .args(["rsa", "-pubout"]) .stdin(Stdio::piped()) @@ -391,6 +395,10 @@ pub fn init_bare_repo( } std::fs::create_dir_all(&repo_dir) .map_err(|e| format!("failed to create git dir {}: {e}", repo_dir.display()))?; + #[expect( + clippy::disallowed_methods, + reason = "This synchronous test harness helper initializes fixture git repositories with the real git CLI." + )] let output = std::process::Command::new("git") .args(["init", "--bare"]) .arg(&repo_dir) @@ -404,6 +412,10 @@ pub fn init_bare_repo( } // Enable http.receivepack so push works via git-http-backend + #[expect( + clippy::disallowed_methods, + reason = "This synchronous test harness helper configures fixture git repositories with the real git CLI." + )] let output = std::process::Command::new("git") .args(["config", "http.receivepack", "true"]) .current_dir(&repo_dir) diff --git a/test/twin/openai/tests/debug_ui.rs b/test/twin/openai/tests/debug_ui.rs index d5f0741c2..c2cf0006c 100644 --- a/test/twin/openai/tests/debug_ui.rs +++ b/test/twin/openai/tests/debug_ui.rs @@ -1,3 +1,8 @@ +#![expect( + clippy::disallowed_methods, + reason = "These browser-debug integration tests synchronously probe for Chrome binaries before launching external tooling." +)] + mod common; use serde_json::json;