From 34d7f6b473185c1b6cace10075f8cb36371f8516 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Sun, 12 Jul 2026 03:44:20 +0530 Subject: [PATCH 1/3] ci: add clippy, fmt, and cargo-deny gates behind a CI Green aggregator Adopts the CI check pattern from lean-ctx's PR #721: cargo clippy (-D warnings, exempting the repo's existing warn-only unsafe_code and clippy::pedantic policy), cargo fmt --check, and cargo-deny (license and dependency policy, deny.toml adopted verbatim from lean-ctx since its allow-list/bans/sources policy applies unchanged here). A CI Green job aggregates build/clippy/fmt/deny so branch protection can point at one stable check instead of every individual leg. Also runs cargo fmt across the whole workspace (first pass, no prior formatting enforcement existed) and fixes every clippy finding the new gate surfaces: real bugs (redundant match guard, needless range loop, unnecessary sort_by, lines().flatten() looping forever on stream errors, duplicated if/else branches, nested format! args), plus targeted #[allow] suppressions for pre-existing dead code and too-many-arguments functions that are out of scope for this change. --- .github/workflows/ci.yml | 84 +++ crates/agent-registry/src/detect.rs | 116 ++-- crates/agent-registry/src/lib.rs | 7 +- crates/agent-registry/src/registry.rs | 9 +- crates/agentflare-artifacts/src/lib.rs | 63 +- crates/agentflare-artifacts/src/server.rs | 36 +- crates/agentflare-artifacts/src/store.rs | 29 +- crates/caveman/src/compress.rs | 103 +++- crates/caveman/src/lib.rs | 2 +- crates/caveman/src/llm.rs | 30 +- crates/caveman/src/prompt.rs | 12 +- crates/caveman/src/sensitive.rs | 26 +- crates/caveman/src/validate.rs | 62 +- crates/gateway-registry/src/audit.rs | 16 +- crates/gateway-registry/src/circuit.rs | 6 +- crates/gateway-registry/src/config.rs | 58 +- crates/gateway-registry/src/db.rs | 64 ++- crates/gateway-registry/src/error.rs | 13 +- crates/gateway-registry/src/lib.rs | 12 +- crates/gateway-registry/src/mcp_http.rs | 28 +- crates/gateway-registry/src/mcp_stdio.rs | 29 +- crates/gateway-registry/src/redact.rs | 19 +- crates/gateway-registry/src/registry.rs | 126 +++- crates/gateway-registry/src/sanitize.rs | 6 +- crates/gateway-registry/src/search.rs | 10 +- crates/gateway-registry/src/truncate.rs | 26 +- .../tests/fixtures/fixture_server.rs | 8 +- .../tests/gateway_audit_log.rs | 39 +- .../gateway-registry/tests/mcp_http_call.rs | 35 +- .../tests/mcp_http_circuit_breaker.rs | 45 +- .../tests/mcp_http_discover.rs | 5 +- .../gateway-registry/tests/mcp_stdio_call.rs | 25 +- .../tests/mcp_stdio_circuit_breaker.rs | 75 ++- .../tests/mcp_stdio_discover.rs | 5 +- .../tests/mcp_stdio_reconnect.rs | 36 +- .../tests/mcp_stdio_timeout.rs | 40 +- crates/gateway-registry/tests/registry.rs | 78 ++- crates/gateway-registry/tests/support/mod.rs | 30 +- crates/ponytail/src/config.rs | 36 +- crates/ponytail/src/detect.rs | 425 +++++++++++--- crates/ponytail/src/instructions.rs | 18 +- crates/ponytail/src/lib.rs | 2 +- crates/ponytail/src/platform.rs | 18 +- crates/ponytail/src/state.rs | 12 +- crates/ponytail/src/sub_skills.rs | 75 ++- crates/ponytail/src/switcher.rs | 46 +- crates/skill-registry/src/db.rs | 41 +- crates/skill-registry/src/frontmatter.rs | 7 +- crates/skill-registry/src/lib.rs | 4 +- crates/skill-registry/src/load.rs | 51 +- crates/skill-registry/src/search.rs | 32 +- crates/skill-registry/src/sources.rs | 99 +++- crates/skill-registry/tests/golden_queries.rs | 52 +- deny.toml | 55 ++ src/agent_install.rs | 26 +- src/agent_launch.rs | 40 +- src/agents.rs | 84 +-- src/alias.rs | 75 ++- src/auth.rs | 292 ++++++---- src/auth_crypt.rs | 14 +- src/auth_db.rs | 25 +- src/auth_runner.rs | 28 +- src/build_time.rs | 9 +- src/channels.rs | 34 +- src/claims.rs | 61 +- src/cli/agents.rs | 11 +- src/cli/auth.rs | 104 +++- src/cli/caveman.rs | 15 +- src/cli/channel.rs | 10 +- src/cli/claim.rs | 59 +- src/cli/coaching.rs | 4 +- src/cli/gateway.rs | 22 +- src/cli/handoff.rs | 22 +- src/cli/memory.rs | 49 +- src/cli/ponytail.rs | 31 +- src/cli/review.rs | 85 ++- src/coaching.rs | 39 +- src/components.rs | 149 +++-- src/cost.rs | 106 +++- src/db.rs | 29 +- src/dev_vars.rs | 3 +- src/errors.rs | 6 + src/gateway_integrations.rs | 28 +- src/gateway_secrets.rs | 32 +- src/hook.rs | 92 ++- src/init.rs | 244 +++++--- src/main.rs | 6 +- src/mcp_prompts.rs | 50 +- src/mcp_server.rs | 540 +++++++++++++----- src/memory/mcp.rs | 121 +++- src/memory/observations.rs | 124 +++- src/memory/prompts.rs | 22 +- src/memory/relations.rs | 41 +- src/memory/search.rs | 57 +- src/memory/sessions.rs | 34 +- src/memory/store.rs | 8 +- src/memory/summaries.rs | 12 +- src/mise_install.rs | 1 + src/optimize.rs | 83 ++- src/pricing.rs | 36 +- src/review.rs | 127 +++- src/rollup.rs | 182 ++++-- src/shell.rs | 9 +- src/state.rs | 10 +- src/tool_install.rs | 10 +- src/uninstall.rs | 169 ++++-- src/update.rs | 32 +- tests/caveman_cli.rs | 23 +- 108 files changed, 4407 insertions(+), 1504 deletions(-) create mode 100644 deny.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e530e293..57c3656e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,3 +68,87 @@ jobs: run: cargo build --workspace --verbose - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' run: cargo test --workspace --verbose + + clippy: + needs: changes + if: always() + timeout-minutes: 15 + runs-on: ubuntu-latest + steps: + - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + with: + fetch-depth: 0 + - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + components: clippy + - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' + uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 # zizmor: ignore[cache-poisoning] save-if already gates writes to master + with: + save-if: ${{ github.ref == 'refs/heads/master' }} + # The root [lints.rust] unsafe_code and sub-crate [lints.clippy] + # pedantic overrides are deliberately warn-only, not deny — exempt both + # groups here so this gate doesn't silently escalate that policy. + - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' + run: cargo clippy --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic + + fmt: + needs: changes + if: always() + timeout-minutes: 10 + runs-on: ubuntu-latest + steps: + - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + with: + fetch-depth: 0 + - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + components: rustfmt + - if: needs.changes.result != 'success' || needs.changes.outputs.code == 'true' + run: cargo fmt --check + + deny: + # No paths filter, same rationale as security-check.yml's audit job: a + # path-filtered required check never reports on PRs that skip it, + # blocking the merge forever. cargo-deny is cheap enough to run always. + name: cargo-deny + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + - uses: EmbarkStudios/cargo-deny-action@5bb39ff5d5a0e94dc9e2dc94eced0c6129743a57 # v2 + with: + manifest-path: Cargo.toml + + # Single aggregating gate. Point branch protection at THIS job instead of + # every leg above — stays correct when jobs are added/removed and can't be + # bypassed by a silently-missing required check. + ci-green: + name: CI Green + if: always() + needs: + - build + - clippy + - fmt + - deny + runs-on: ubuntu-latest + steps: + - name: Verify no required job failed + shell: bash + env: + NEEDS_JSON: ${{ toJSON(needs) }} + run: | + echo "$NEEDS_JSON" + python3 - <<'PY' + import json, os, sys + + needs = json.loads(os.environ["NEEDS_JSON"]) + blocked = {k: v["result"] for k, v in needs.items() + if v["result"] in ("failure", "cancelled")} + if blocked: + print("Required CI jobs did not pass:", blocked) + sys.exit(1) + print("All required CI jobs passed (success or skipped).") + PY diff --git a/crates/agent-registry/src/detect.rs b/crates/agent-registry/src/detect.rs index 3e89e50d..b5f4a609 100644 --- a/crates/agent-registry/src/detect.rs +++ b/crates/agent-registry/src/detect.rs @@ -28,7 +28,7 @@ pub static PATH_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); /// the approach used by `agents-cli`'s `findInPath` and `caam`'s /// `findBinary`, minus their shims-dir exclusion (agentflare has no shims /// directory yet). -#[must_use] +#[must_use] pub fn find_binary(names: &[&str]) -> Option { let path_var = std::env::var_os("PATH")?; for dir in std::env::split_paths(&path_var) { @@ -58,7 +58,9 @@ mod find_binary_tests { // paths::test_support::GLOBAL_STATE_LOCK. fn with_temp_path_dir(f: impl FnOnce(&Path)) { - let _guard = super::PATH_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _guard = super::PATH_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let dir = std::env::temp_dir().join(format!("agentflare-test-path-{}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); @@ -66,17 +68,17 @@ mod find_binary_tests { unsafe { // SAFETY: PATH_LOCK mutex serializes all PATH mutations; // no other thread can read or write PATH concurrently. - std::env::set_var("PATH", &dir) + std::env::set_var("PATH", &dir); }; f(&dir); match original { Some(p) => unsafe { // SAFETY: PATH_LOCK mutex serializes all PATH mutations. - std::env::set_var("PATH", p) + std::env::set_var("PATH", p); }, None => unsafe { // SAFETY: PATH_LOCK mutex serializes all PATH mutations. - std::env::remove_var("PATH") + std::env::remove_var("PATH"); }, } let _ = std::fs::remove_dir_all(&dir); @@ -111,7 +113,7 @@ mod find_binary_tests { /// Find the first `\d+\.\d+\.\d+`-shaped substring in `text` (a `--version` /// command's combined stdout+stderr). Hand-rolled instead of pulling in the /// `regex` crate for one pattern. -#[must_use] +#[must_use] pub fn extract_version(text: &str) -> Option { let chars: Vec = text.chars().collect(); for start in 0..chars.len() { @@ -145,12 +147,18 @@ mod extract_version_tests { #[test] fn extracts_version_from_prefixed_output() { - assert_eq!(extract_version("claude-code/1.2.3"), Some("1.2.3".to_string())); + assert_eq!( + extract_version("claude-code/1.2.3"), + Some("1.2.3".to_string()) + ); } #[test] fn extracts_version_embedded_in_a_sentence() { - assert_eq!(extract_version("codex cli version 0.128.0 (build abc)"), Some("0.128.0".to_string())); + assert_eq!( + extract_version("codex cli version 0.128.0 (build abc)"), + Some("0.128.0".to_string()) + ); } #[test] @@ -165,7 +173,10 @@ mod extract_version_tests { #[test] fn returns_first_match_when_multiple_numbers_present() { - assert_eq!(extract_version("built with node 20.11.0 for app 1.2.3"), Some("20.11.0".to_string())); + assert_eq!( + extract_version("built with node 20.11.0 for app 1.2.3"), + Some("20.11.0".to_string()) + ); } } @@ -220,7 +231,10 @@ fn run_version_command(binary: &Path, args: &[&str]) -> Result { Ok(Ok(text)) if !text.trim().is_empty() => Ok(text), Ok(Ok(_)) => Err(format!("{} produced no output", binary.display())), Ok(Err(e)) => Err(e), - Err(_) => Err(format!("{} timed out after {VERSION_TIMEOUT:?}", binary.display())), + Err(_) => Err(format!( + "{} timed out after {VERSION_TIMEOUT:?}", + binary.display() + )), } } @@ -243,9 +257,11 @@ pub fn resolve_version_with( let binary_path_str = binary_path.to_string_lossy().into_owned(); if let Some(entry) = cache.get(agent_key) - && entry.binary_path == binary_path_str && entry.mtime == mtime { - return Ok(entry.version.clone()); - } + && entry.binary_path == binary_path_str + && entry.mtime == mtime + { + return Ok(entry.version.clone()); + } let raw = runner.run(binary_path, version_args)?; let version = extract_version(&raw) @@ -253,7 +269,11 @@ pub fn resolve_version_with( cache.insert( agent_key.to_string(), - VersionCacheEntry { binary_path: binary_path_str, mtime, version: version.clone() }, + VersionCacheEntry { + binary_path: binary_path_str, + mtime, + version: version.clone(), + }, ); Ok(version) } @@ -265,7 +285,13 @@ pub fn resolve_version( version_args: &[&str], cache: &mut HashMap, ) -> Result { - resolve_version_with(&RealVersionRunner, agent_key, binary_path, version_args, cache) + resolve_version_with( + &RealVersionRunner, + agent_key, + binary_path, + version_args, + cache, + ) } #[cfg(test)] @@ -286,7 +312,8 @@ mod resolve_version_tests { /// A real file on disk so `fs::metadata` succeeds — its content is /// irrelevant since `FakeRunner` never actually executes it. fn temp_binary_file(name: &str) -> PathBuf { - let dir = std::env::temp_dir().join(format!("agentflare-test-resolve-{}", std::process::id())); + let dir = + std::env::temp_dir().join(format!("agentflare-test-resolve-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let path = dir.join(name); let mut f = std::fs::File::create(&path).unwrap(); @@ -297,10 +324,13 @@ mod resolve_version_tests { #[test] fn cache_miss_spawns_and_caches_result() { let binary = temp_binary_file("agent-a"); - let runner = FakeRunner { response: Ok("agent-a version 1.2.3".to_string()) }; + let runner = FakeRunner { + response: Ok("agent-a version 1.2.3".to_string()), + }; let mut cache = HashMap::new(); - let version = resolve_version_with(&runner, "agent-a", &binary, &["--version"], &mut cache).unwrap(); + let version = + resolve_version_with(&runner, "agent-a", &binary, &["--version"], &mut cache).unwrap(); assert_eq!(version, "1.2.3"); assert_eq!(cache.get("agent-a").unwrap().version, "1.2.3"); @@ -309,8 +339,13 @@ mod resolve_version_tests { #[test] fn cache_hit_does_not_call_runner_again() { let binary = temp_binary_file("agent-b"); - let mtime = std::fs::metadata(&binary).unwrap().modified().unwrap() - .duration_since(std::time::UNIX_EPOCH).unwrap().as_secs(); + let mtime = std::fs::metadata(&binary) + .unwrap() + .modified() + .unwrap() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); let mut cache = HashMap::new(); cache.insert( "agent-b".to_string(), @@ -328,7 +363,9 @@ mod resolve_version_tests { } } - let version = resolve_version_with(&PanicRunner, "agent-b", &binary, &["--version"], &mut cache).unwrap(); + let version = + resolve_version_with(&PanicRunner, "agent-b", &binary, &["--version"], &mut cache) + .unwrap(); assert_eq!(version, "9.9.9"); } @@ -344,28 +381,38 @@ mod resolve_version_tests { version: "0.0.0".to_string(), }, ); - let runner = FakeRunner { response: Ok("2.0.0".to_string()) }; + let runner = FakeRunner { + response: Ok("2.0.0".to_string()), + }; - let version = resolve_version_with(&runner, "agent-c", &binary, &["--version"], &mut cache).unwrap(); + let version = + resolve_version_with(&runner, "agent-c", &binary, &["--version"], &mut cache).unwrap(); assert_eq!(version, "2.0.0"); } #[test] fn failed_resolution_is_not_persisted_to_cache() { let binary = temp_binary_file("agent-d"); - let runner = FakeRunner { response: Err("boom".to_string()) }; + let runner = FakeRunner { + response: Err("boom".to_string()), + }; let mut cache = HashMap::new(); let result = resolve_version_with(&runner, "agent-d", &binary, &["--version"], &mut cache); assert!(result.is_err()); - assert!(!cache.contains_key("agent-d"), "a failed resolution must not be cached"); + assert!( + !cache.contains_key("agent-d"), + "a failed resolution must not be cached" + ); } #[test] fn unparseable_success_output_is_not_persisted_to_cache() { let binary = temp_binary_file("agent-e"); - let runner = FakeRunner { response: Ok("no version information here".to_string()) }; + let runner = FakeRunner { + response: Ok("no version information here".to_string()), + }; let mut cache = HashMap::new(); let result = resolve_version_with(&runner, "agent-e", &binary, &["--version"], &mut cache); @@ -382,7 +429,9 @@ mod resolve_version_tests { // Take the shared PATH_LOCK so this can't run concurrently with a // find_binary_tests/detect_all_tests test that has repointed PATH // to a temp-only directory — this test needs the real PATH intact. - let _guard = PATH_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _guard = PATH_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); // The one real-spawn test: `rustc` is guaranteed on PATH inside any // `cargo test` invocation, so this is portable without a stub binary. let output = run_version_command(Path::new("rustc"), &["--version"]).unwrap(); @@ -466,25 +515,28 @@ mod detect_all_tests { } fn with_temp_path_dir(f: impl FnOnce(&Path)) { - let _guard = super::PATH_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - let dir = std::env::temp_dir().join(format!("agentflare-test-detect-all-{}", std::process::id())); + let _guard = super::PATH_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let dir = + std::env::temp_dir().join(format!("agentflare-test-detect-all-{}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); let original = std::env::var_os("PATH"); unsafe { // SAFETY: PATH_LOCK mutex serializes all PATH mutations; // no other thread can read or write PATH concurrently. - std::env::set_var("PATH", &dir) + std::env::set_var("PATH", &dir); }; f(&dir); match original { Some(p) => unsafe { // SAFETY: PATH_LOCK mutex serializes all PATH mutations. - std::env::set_var("PATH", p) + std::env::set_var("PATH", p); }, None => unsafe { // SAFETY: PATH_LOCK mutex serializes all PATH mutations. - std::env::remove_var("PATH") + std::env::remove_var("PATH"); }, } let _ = std::fs::remove_dir_all(&dir); diff --git a/crates/agent-registry/src/lib.rs b/crates/agent-registry/src/lib.rs index 33e6a695..5d188db3 100644 --- a/crates/agent-registry/src/lib.rs +++ b/crates/agent-registry/src/lib.rs @@ -1,4 +1,7 @@ pub mod detect; pub mod registry; -pub use registry::{headless_args, Agent, AgentSpec, Tier, REGISTRY, spec}; -pub use detect::{detect_all, detect_all_with, find_binary, resolve_version, resolve_version_with, DetectedAgent, VersionRunner, RealVersionRunner, VersionCacheEntry}; +pub use detect::{ + DetectedAgent, RealVersionRunner, VersionCacheEntry, VersionRunner, detect_all, + detect_all_with, find_binary, resolve_version, resolve_version_with, +}; +pub use registry::{Agent, AgentSpec, REGISTRY, Tier, headless_args, spec}; diff --git a/crates/agent-registry/src/registry.rs b/crates/agent-registry/src/registry.rs index fd08625a..77b08338 100644 --- a/crates/agent-registry/src/registry.rs +++ b/crates/agent-registry/src/registry.rs @@ -30,7 +30,7 @@ pub enum Agent { } impl Agent { - #[must_use] + #[must_use] pub fn as_str(self) -> &'static str { match self { Agent::ClaudeCode => "claude-code", @@ -278,7 +278,7 @@ pub static REGISTRY: &[AgentSpec] = &[ /// Not yet consumed outside tests — wired up by the upcoming agent detection /// engine and CLI commands. #[allow(dead_code)] -#[must_use] +#[must_use] pub fn spec(agent: Agent) -> &'static AgentSpec { REGISTRY .iter() @@ -314,7 +314,10 @@ mod tests { #[test] fn registry_has_seventeen_cli_tier_and_three_extension_tier() { let cli_count = REGISTRY.iter().filter(|s| s.tier == Tier::Cli).count(); - let ext_count = REGISTRY.iter().filter(|s| s.tier == Tier::Extension).count(); + let ext_count = REGISTRY + .iter() + .filter(|s| s.tier == Tier::Extension) + .count(); assert_eq!(cli_count, 17); assert_eq!(ext_count, 3); } diff --git a/crates/agentflare-artifacts/src/lib.rs b/crates/agentflare-artifacts/src/lib.rs index 1ee9007b..d4ac389c 100644 --- a/crates/agentflare-artifacts/src/lib.rs +++ b/crates/agentflare-artifacts/src/lib.rs @@ -30,7 +30,9 @@ mod tests { use std::io::Write; stream.write_all(req.as_bytes()).unwrap(); stream.flush().unwrap(); - stream.set_read_timeout(Some(Duration::from_secs(2))).unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(2))) + .unwrap(); let mut reader = BufReader::new(&stream); let mut full = String::new(); let _ = reader.read_to_string(&mut full); @@ -158,10 +160,16 @@ mod tests { .unwrap(); let listing = read_http("/", port); - assert!(listing.contains("http-test"), "index page shows artifact: {listing}"); + assert!( + listing.contains("http-test"), + "index page shows artifact: {listing}" + ); let resp = read_http("/", port); - assert!(resp.contains("HTTP/1.0 200") || resp.contains("HTTP/1.1 200"), "bad status: {resp}"); + assert!( + resp.contains("HTTP/1.0 200") || resp.contains("HTTP/1.1 200"), + "bad status: {resp}" + ); } #[test] @@ -360,11 +368,20 @@ mod tests { .unwrap(); let page = read_http(&format!("/{}", resp.id), port); - assert!(!page.contains("