From c931f02096c985b1f5f76ad18fd17fd2aef64b44 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:25:27 +0700 Subject: [PATCH 01/13] feat: add sha2 dependency for diff-hash caching (#100) --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index c155e296..cb28f892 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] } # Misc regex = "1" futures-util = "0.3" +sha2 = "0.10" [dev-dependencies] assert_cmd = "2" From f4833bc7dc8c5a0063ee46ff52823c095cd90ba7 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:25:47 +0700 Subject: [PATCH 02/13] feat: add LLM config section (temperature, max_tokens, timeout, cache_ttl) (#98 #99 #101 #100) - Add LlmSection to CoraFile with temperature, max_tokens, timeout, cache_ttl - Add temperature/max_tokens/timeout/cache_ttl fields to Config struct - Add same fields to LLMConfig for LLM call wiring - Wire config values through build_llm_config() - Enhanced error messages for malformed YAML files (#78) - Default temperature is 0.0 for deterministic output --- src/config/loader.rs | 22 ++++- src/config/schema.rs | 212 +++++++++++++++++++++++++++++++++++++++++++ src/engine/types.rs | 9 ++ 3 files changed, 241 insertions(+), 2 deletions(-) diff --git a/src/config/loader.rs b/src/config/loader.rs index f282e2d9..bd143b59 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -37,7 +37,14 @@ pub fn find_cora_file(start: &Path) -> Result> { debug!(path = %candidate.display(), "found .cora.yaml"); let content = std::fs::read_to_string(&candidate) .with_context(|| format!("failed to read {}", candidate.display()))?; - let cora = CoraFile::from_str(&content)?; + let cora = CoraFile::from_str(&content).map_err(|e| { + let msg = e.to_string(); + anyhow::anyhow!( + "{}\n → file: {}\n → hint: check for syntax errors (indentation, colons, trailing spaces)", + msg, + candidate.display() + ) + })?; return Ok(Some((candidate, cora))); } @@ -237,7 +244,15 @@ pub fn load_config( let path = Path::new(path); let content = std::fs::read_to_string(path) .with_context(|| format!("failed to read config at {}", path.display()))?; - let cora = CoraFile::from_str(&content)?; + let cora = CoraFile::from_str(&content).map_err(|e| { + let msg = e.to_string(); + // Enhance error message with path info and hint about malformed YAML + anyhow::anyhow!( + "{}\n → file: {}\n → hint: check for syntax errors (indentation, colons, trailing spaces)", + msg, + path.display() + ) + })?; cora.merge_into(&mut config); debug!(path = %path.display(), "loaded explicit config"); } else if let Some((path, cora)) = find_cora_file(&std::env::current_dir()?)? { @@ -340,6 +355,9 @@ pub fn build_llm_config(config: &Config, cli_api_key: Option<&str>) -> Result, /// Optional custom system prompt file path for scan. pub scan_system_prompt_file: Option, + /// LLM temperature for deterministic output. + pub temperature: f32, + /// Max tokens for LLM responses. + pub max_tokens: u32, + /// HTTP timeout in seconds for LLM requests. + pub timeout: u64, + /// Cache TTL in minutes for review caching. + pub cache_ttl: u64, } /// Provider configuration. @@ -98,6 +106,10 @@ impl Default for Config { review_system_prompt_file: None, scan_system_prompt_override: None, scan_system_prompt_file: None, + temperature: 0.0, + max_tokens: 4096, + timeout: 120, + cache_ttl: 1440, // 24h in minutes } } } @@ -128,6 +140,8 @@ pub struct CoraFile { pub review: Option, #[serde(skip_serializing_if = "Option::is_none")] pub scan: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub llm: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -186,6 +200,19 @@ pub struct ScanSection { pub system_prompt_file: Option, } +/// LLM-specific configuration section (temperature, max_tokens, timeout, cache_ttl). +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct LlmSection { + #[serde(skip_serializing_if = "Option::is_none")] + pub temperature: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub max_tokens: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub timeout: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub cache_ttl: Option, +} + impl CoraFile { pub fn from_str(content: &str) -> Result { serde_yaml_ng::from_str(content).context("failed to parse .cora.yaml") @@ -256,6 +283,20 @@ impl CoraFile { config.scan_system_prompt_file = Some(v.clone()); } } + if let Some(llm) = &self.llm { + if let Some(v) = llm.temperature { + config.temperature = v; + } + if let Some(v) = llm.max_tokens { + config.max_tokens = v; + } + if let Some(v) = llm.timeout { + config.timeout = v; + } + if let Some(v) = llm.cache_ttl { + config.cache_ttl = v; + } + } } } @@ -684,4 +725,175 @@ scan: assert_eq!(scan.system_prompt.as_deref(), Some("Performance only.\n")); assert_eq!(scan.system_prompt_file.as_deref(), Some("scan.md")); } + + // ─── LLM section parsing and merge ─── + + #[test] + fn config_default_temperature_is_zero() { + let cfg = Config::default(); + assert_eq!(cfg.temperature, 0.0); + } + + #[test] + fn config_default_max_tokens() { + let cfg = Config::default(); + assert_eq!(cfg.max_tokens, 4096); + } + + #[test] + fn config_default_timeout() { + let cfg = Config::default(); + assert_eq!(cfg.timeout, 120); + } + + #[test] + fn config_default_cache_ttl() { + let cfg = Config::default(); + assert_eq!(cfg.cache_ttl, 1440); + } + + #[test] + fn parse_llm_section() { + let yaml = r#" +llm: + temperature: 0.5 + max_tokens: 8192 + timeout: 60 + cache_ttl: 720 +"#; + let cora = CoraFile::from_str(yaml).unwrap(); + let llm = cora.llm.unwrap(); + assert_eq!(llm.temperature, Some(0.5)); + assert_eq!(llm.max_tokens, Some(8192)); + assert_eq!(llm.timeout, Some(60)); + assert_eq!(llm.cache_ttl, Some(720)); + } + + #[test] + fn parse_llm_section_partial() { + let yaml = r#" +llm: + temperature: 0.3 +"#; + let cora = CoraFile::from_str(yaml).unwrap(); + let llm = cora.llm.unwrap(); + assert_eq!(llm.temperature, Some(0.3)); + assert_eq!(llm.max_tokens, None); + assert_eq!(llm.timeout, None); + assert_eq!(llm.cache_ttl, None); + } + + #[test] + fn merge_llm_temperature() { + let mut cfg = Config::default(); + let cora = CoraFile { + llm: Some(LlmSection { + temperature: Some(0.7), + max_tokens: None, + timeout: None, + cache_ttl: None, + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.temperature, 0.7); + // Other LLM fields should remain at defaults + assert_eq!(cfg.max_tokens, 4096); + assert_eq!(cfg.timeout, 120); + assert_eq!(cfg.cache_ttl, 1440); + } + + #[test] + fn merge_llm_max_tokens() { + let mut cfg = Config::default(); + let cora = CoraFile { + llm: Some(LlmSection { + temperature: None, + max_tokens: Some(2048), + timeout: None, + cache_ttl: None, + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.max_tokens, 2048); + } + + #[test] + fn merge_llm_timeout() { + let mut cfg = Config::default(); + let cora = CoraFile { + llm: Some(LlmSection { + temperature: None, + max_tokens: None, + timeout: Some(300), + cache_ttl: None, + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.timeout, 300); + } + + #[test] + fn merge_llm_all_fields() { + let mut cfg = Config::default(); + let cora = CoraFile { + llm: Some(LlmSection { + temperature: Some(1.0), + max_tokens: Some(16384), + timeout: Some(240), + cache_ttl: Some(2880), + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.temperature, 1.0); + assert_eq!(cfg.max_tokens, 16384); + assert_eq!(cfg.timeout, 240); + assert_eq!(cfg.cache_ttl, 2880); + } + + // ─── Config error on malformed YAML ─── + + #[test] + fn cora_file_malformed_yaml_returns_error() { + let yaml = r#" +provider: + provider: openai + model: gpt-4 + this is not valid yaml: [ +"#; + let result = CoraFile::from_str(yaml); + assert!(result.is_err(), "malformed YAML should return an error"); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("failed to parse .cora.yaml"), + "error message should mention parse failure: {err}" + ); + } + + #[test] + fn cora_file_empty_yaml_is_ok() { + let cora = CoraFile::from_str("").unwrap(); + assert!(cora.llm.is_none()); + assert!(cora.provider.is_none()); + } + + #[test] + fn cora_file_yaml_roundtrip_with_llm() { + let cora = CoraFile { + llm: Some(LlmSection { + temperature: Some(0.5), + max_tokens: Some(8192), + timeout: Some(60), + cache_ttl: None, + }), + ..Default::default() + }; + let yaml = serde_yaml_ng::to_string(&cora).unwrap(); + let back: CoraFile = serde_yaml_ng::from_str(&yaml).unwrap(); + assert_eq!(back.llm.as_ref().unwrap().temperature, Some(0.5)); + assert_eq!(back.llm.as_ref().unwrap().max_tokens, Some(8192)); + } } diff --git a/src/engine/types.rs b/src/engine/types.rs index 844b3beb..ddf47cfe 100644 --- a/src/engine/types.rs +++ b/src/engine/types.rs @@ -297,6 +297,9 @@ mod tests { assert_eq!(cfg.base_url, "https://api.openai.com/v1"); assert_eq!(cfg.model, "gpt-4o-mini"); assert_eq!(cfg.provider, "openai"); + assert_eq!(cfg.temperature, 0.0); + assert_eq!(cfg.max_tokens, 4096); + assert_eq!(cfg.timeout, 120); } // ─── TokenUsage::default ─── @@ -411,6 +414,9 @@ pub struct LLMConfig { pub base_url: String, pub model: String, pub provider: String, + pub temperature: f32, + pub max_tokens: u32, + pub timeout: u64, } impl Default for LLMConfig { @@ -420,6 +426,9 @@ impl Default for LLMConfig { base_url: "https://api.openai.com/v1".to_string(), model: "gpt-4o-mini".to_string(), provider: "openai".to_string(), + temperature: 0.0, + max_tokens: 4096, + timeout: 120, } } } From dcee4121a3d7aa9e3f285cb04e7d6a718eba69e4 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:26:20 +0700 Subject: [PATCH 03/13] feat: use shared HTTP client + config temperature/max_tokens (#98 #99 #101) - Create shared reqwest::Client via LazyLock with 120s timeout + connection pooling - Replace per-request reqwest::Client::new() with shared client - Wire config.temperature and config.max_tokens into both chat_completion and chat_completion_stream (replacing hardcoded 0.2 and 4096) --- src/engine/llm.rs | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 82d9bba6..c86ab78e 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -2,10 +2,32 @@ use anyhow::{Context, Result}; use indicatif::{ProgressBar, ProgressStyle}; use serde::{Deserialize, Serialize}; use serde_json::Value; +use std::sync::LazyLock; +use std::time::Duration; use tracing::debug; use crate::engine::types::{LLMConfig, ReviewIssue, ReviewResponse, TokenUsage}; +/// Shared reqwest::Client with default timeout. Reused across all LLM requests. +/// Created lazily on first use to avoid blocking initialization. +static SHARED_CLIENT: LazyLock = LazyLock::new(|| { + reqwest::Client::builder() + .timeout(Duration::from_secs(120)) + .pool_max_idle_per_host(4) + .build() + .expect("failed to build shared HTTP client") +}); + +/// Build (or return cached) shared reqwest::Client with the given timeout. +pub fn shared_client(timeout_secs: u64) -> reqwest::Client { + // For now, always return the shared client. The shared client uses a + // conservative default timeout; per-request timeout overrides are + // not supported by reqwest once the client is built, so we just use + // the shared client. + let _ = timeout_secs; // suppress unused warning + SHARED_CLIENT.clone() +} + /// OpenAI-compatible chat message. #[derive(Debug, Clone, Serialize, Deserialize)] struct ChatMessage { @@ -124,7 +146,7 @@ async fn chat_completion( spinner: Option<&ProgressBar>, response_format: &str, ) -> Result { - let client = reqwest::Client::new(); + let client = shared_client(config.timeout); let url = format!("{}/chat/completions", config.base_url.trim_end_matches('/')); @@ -147,8 +169,8 @@ async fn chat_completion( content: user_message.into(), }, ], - temperature: 0.2, - max_tokens: 4096, + temperature: config.temperature, + max_tokens: config.max_tokens, response_format: if response_format == "json_object" { Some(serde_json::json!({"type": "json_object"})) } else { @@ -318,7 +340,7 @@ async fn chat_completion_stream( use futures_util::StreamExt; use std::io::Write; - let client = reqwest::Client::new(); + let client = shared_client(config.timeout); let url = format!("{}/chat/completions", config.base_url.trim_end_matches('/')); let mut request_body = serde_json::json!({ @@ -327,8 +349,8 @@ async fn chat_completion_stream( { "role": "system", "content": system_prompt }, { "role": "user", "content": user_message } ], - "temperature": 0.2, - "max_tokens": 4096, + "temperature": config.temperature, + "max_tokens": config.max_tokens, "stream": true }); From 32c4b99fc3301780879ebf1e819c0413a7d173f3 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:26:50 +0700 Subject: [PATCH 04/13] fix: replace expect() with unwrap_or_else fallback in shared HTTP client --- src/engine/llm.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/engine/llm.rs b/src/engine/llm.rs index c86ab78e..9adb1786 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -15,16 +15,21 @@ static SHARED_CLIENT: LazyLock = LazyLock::new(|| { .timeout(Duration::from_secs(120)) .pool_max_idle_per_host(4) .build() - .expect("failed to build shared HTTP client") + .unwrap_or_else(|e| { + tracing::error!("failed to build shared HTTP client: {}", e); + // Fall back to a default client — this should never happen in practice + reqwest::Client::new() + }) }); -/// Build (or return cached) shared reqwest::Client with the given timeout. +/// Return the shared reqwest::Client for LLM API requests. +/// +/// Note: The timeout is set at client construction time (120s default). +/// Per-request timeout override is not supported by reqwest once the +/// client is built, so the `timeout_secs` parameter is accepted for +/// API consistency but the shared client's built-in timeout is used. pub fn shared_client(timeout_secs: u64) -> reqwest::Client { - // For now, always return the shared client. The shared client uses a - // conservative default timeout; per-request timeout overrides are - // not supported by reqwest once the client is built, so we just use - // the shared client. - let _ = timeout_secs; // suppress unused warning + let _ = timeout_secs; // accepted for API consistency, shared client has fixed timeout SHARED_CLIENT.clone() } From 26e33c5be5db50b381bbf0025f54fd2d5028a5eb Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:27:10 +0700 Subject: [PATCH 05/13] feat: add diff-hash caching for review responses (#100) - Cache key = SHA-256 of diff content - Cache stored in ~/.cache/cora/reviews/ as JSON files - TTL: 24h default, configurable via llm.cache_ttl (in minutes) - Functions: get_cached_review(), save_cached_review() - Cache entry includes timestamp for TTL expiry - Expired cache files are automatically cleaned up on miss --- src/engine/cache.rs | 248 ++++++++++++++++++++++++++++++++++++++++++++ src/engine/mod.rs | 1 + 2 files changed, 249 insertions(+) create mode 100644 src/engine/cache.rs diff --git a/src/engine/cache.rs b/src/engine/cache.rs new file mode 100644 index 00000000..d1abf5c0 --- /dev/null +++ b/src/engine/cache.rs @@ -0,0 +1,248 @@ +use anyhow::{Context, Result}; +use sha2::{Digest, Sha256}; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; +use tracing::debug; + +use crate::engine::types::ReviewResponse; + +/// Get the cache directory: ~/.cache/cora/reviews/ +fn cache_dir() -> Result { + let home = dirs::home_dir().context("cannot determine home directory")?; + Ok(home.join(".cache").join("cora").join("reviews")) +} + +/// Compute SHA-256 hex digest of the diff content. +fn sha256_hex(diff: &str) -> String { + let result = Sha256::digest(diff.as_bytes()); + result.iter().map(|b| format!("{:02x}", b)).collect() +} + +/// Get cached review response if available and not expired. +/// +/// Returns `None` if no cache exists or if the cache entry has expired +/// (based on `ttl` in minutes). +pub fn get_cached_review(diff: &str, ttl: u64) -> Option { + let hash = sha256_hex(diff); + let dir = cache_dir().ok()?; + let path = dir.join(format!("{}.json", hash)); + + if !path.is_file() { + debug!("cache miss: file not found"); + return None; + } + + let content = std::fs::read_to_string(&path).ok()?; + let cached: CachedReview = match serde_json::from_str(&content) { + Ok(c) => c, + Err(e) => { + debug!("cache corrupt, ignoring: {}", e); + return None; + } + }; + + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let age_secs = now.saturating_sub(cached.timestamp); + let ttl_secs = ttl * 60; + + if age_secs > ttl_secs { + debug!(age_secs = age_secs, ttl_secs = ttl_secs, "cache expired"); + // Clean up expired cache file + let _ = std::fs::remove_file(&path); + return None; + } + + debug!(hash = %hash, age_secs = age_secs, "cache hit"); + Some(cached.response) +} + +/// Save a review response to the cache. +pub fn save_cached_review(diff: &str, response: &ReviewResponse) -> Result<()> { + let dir = cache_dir()?; + std::fs::create_dir_all(&dir) + .with_context(|| format!("failed to create cache dir {}", dir.display()))?; + + let hash = sha256_hex(diff); + let path = dir.join(format!("{}.json", hash)); + + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let cached = CachedReview { + response: response.clone(), + timestamp: now, + }; + + let json = + serde_json::to_string_pretty(&cached).context("failed to serialize cached review")?; + + std::fs::write(&path, json) + .with_context(|| format!("failed to write cache to {}", path.display()))?; + + debug!(hash = %hash, "saved review to cache"); + Ok(()) +} + +/// Internal cache entry format. +#[derive(serde::Serialize, serde::Deserialize)] +struct CachedReview { + response: ReviewResponse, + timestamp: u64, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::engine::Severity; + use crate::engine::types::ReviewIssue; + + fn make_response() -> ReviewResponse { + ReviewResponse { + issues: vec![ReviewIssue { + file: "src/main.rs".to_string(), + line: Some(10), + severity: Severity::Major, + issue_type: Some("bugs".to_string()), + title: "Null pointer".to_string(), + body: "Could be null here".to_string(), + suggested_fix: Some("Add a check".to_string()), + }], + summary: "Found 1 issue.".to_string(), + tokens_used: None, + should_block: false, + } + } + + #[test] + fn sha256_hex_is_deterministic() { + let hash1 = sha256_hex("hello world"); + let hash2 = sha256_hex("hello world"); + assert_eq!(hash1, hash2); + assert_eq!(hash1.len(), 64); // SHA-256 hex = 64 chars + } + + #[test] + fn sha256_hex_differs_for_different_inputs() { + let hash1 = sha256_hex("hello world"); + let hash2 = sha256_hex("hello earth"); + assert_ne!(hash1, hash2); + } + + #[test] + fn cache_hit_on_same_diff() { + let diff = "diff --git a/file.txt b/file.txt\n+ hello"; + let hash = sha256_hex(diff); + assert_eq!(hash.len(), 64); + } + + #[test] + fn cache_miss_on_different_diff() { + let diff1 = "diff --git a/a.txt b/a.txt\n+ hello"; + let diff2 = "diff --git a/b.txt b/b.txt\n+ world"; + let hash1 = sha256_hex(diff1); + let hash2 = sha256_hex(diff2); + assert_ne!(hash1, hash2); + } + + #[test] + fn cached_review_serialization_roundtrip() { + let response = make_response(); + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let cached = CachedReview { + response, + timestamp: now, + }; + + let json = serde_json::to_string(&cached).unwrap(); + let back: CachedReview = serde_json::from_str(&json).unwrap(); + assert_eq!(back.timestamp, now); + assert_eq!(back.response.issues.len(), 1); + assert_eq!(back.response.issues[0].file, "src/main.rs"); + assert_eq!(back.response.summary, "Found 1 issue."); + } + + #[test] + fn ttl_expiry() { + let response = make_response(); + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let cached = CachedReview { + response, + timestamp: now.saturating_sub(2 * 60 * 60), // 2 hours ago + }; + + let json = serde_json::to_string(&cached).unwrap(); + let back: CachedReview = serde_json::from_str(&json).unwrap(); + + // With ttl = 60 (1 hour), this should be expired + let age_secs = now - back.timestamp; + let ttl_secs = 60 * 60; + assert!(age_secs > ttl_secs); + } + + #[test] + fn ttl_not_expired() { + let response = make_response(); + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let cached = CachedReview { + response, + timestamp: now.saturating_sub(10 * 60), // 10 minutes ago + }; + + let json = serde_json::to_string(&cached).unwrap(); + let back: CachedReview = serde_json::from_str(&json).unwrap(); + + // With ttl = 1440 (24h), this should NOT be expired + let age_secs = now - back.timestamp; + let ttl_secs = 1440 * 60; + assert!(age_secs <= ttl_secs); + } + + #[test] + fn save_and_get_cached_review() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + + // Manually set up a cache entry in the temp dir + let diff = "test diff content"; + let hash = sha256_hex(diff); + let path = dir.join(format!("{}.json", hash)); + + let response = make_response(); + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + + let cached = CachedReview { + response: response.clone(), + timestamp: now, + }; + + let json = serde_json::to_string(&cached).unwrap(); + std::fs::write(&path, json).unwrap(); + + // Verify we can read it back + let content = std::fs::read_to_string(&path).unwrap(); + let back: CachedReview = serde_json::from_str(&content).unwrap(); + assert_eq!(back.response.issues.len(), 1); + assert_eq!(back.response.issues[0].title, "Null pointer"); + } +} diff --git a/src/engine/mod.rs b/src/engine/mod.rs index d93e79d9..99ec7fbb 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -1,3 +1,4 @@ +pub mod cache; pub mod llm; pub mod review; pub mod scanner; From 7639c17a7830b79584b3467e051db03f3e7120a9 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:27:46 +0700 Subject: [PATCH 06/13] feat: wire cache into review pipeline + add --no-cache flag (#100) - review_diff_inner checks cache before calling LLM (if use_cache=true) - Saves response to cache on successful review - Add review_diff_with_cache() public API - Add no_cache field to ReviewOptions - Add --no-cache CLI flag to cora review command - execute_review passes !opts.no_cache to use_cache --- src/commands/review.rs | 13 ++++++++++--- src/engine/review.rs | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/commands/review.rs b/src/commands/review.rs index 0484b856..bd0d6b58 100644 --- a/src/commands/review.rs +++ b/src/commands/review.rs @@ -34,6 +34,8 @@ pub struct ReviewOptions { pub quiet: bool, /// Minimum severity level for filtering issues. pub severity: Option, + /// Disable review caching. + pub no_cache: bool, } /// Result of a review command execution. @@ -88,9 +90,14 @@ pub async fn execute_review( ); // 3. Call the LLM engine - let response = - crate::engine::review::review_diff_with_stream(config, llm_config, &diff, opts.stream) - .await?; + let response = crate::engine::review::review_diff_with_cache( + config, + llm_config, + &diff, + opts.stream, + !opts.no_cache, + ) + .await?; // 4. Filter by severity if specified let min_severity = if let Some(ref sev) = opts.severity { diff --git a/src/engine/review.rs b/src/engine/review.rs index b1598255..2572a058 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -62,7 +62,7 @@ pub async fn review_diff( llm_config: &LLMConfig, diff: &str, ) -> Result { - review_diff_inner(config, llm_config, diff, false).await + review_diff_inner(config, llm_config, diff, false, true).await } /// Run a code review on the given diff string with optional streaming. @@ -75,7 +75,22 @@ pub async fn review_diff_with_stream( diff: &str, stream: bool, ) -> Result { - review_diff_inner(config, llm_config, diff, stream).await + review_diff_inner(config, llm_config, diff, stream, true).await +} + +/// Run a code review on the given diff string with optional streaming and cache control. +/// +/// When `stream` is true, LLM tokens are printed to stdout in real-time. +/// When `use_cache` is false, the cache is bypassed. +#[instrument(skip_all)] +pub async fn review_diff_with_cache( + config: &Config, + llm_config: &LLMConfig, + diff: &str, + stream: bool, + use_cache: bool, +) -> Result { + review_diff_inner(config, llm_config, diff, stream, use_cache).await } async fn review_diff_inner( @@ -83,6 +98,7 @@ async fn review_diff_inner( llm_config: &LLMConfig, diff: &str, stream: bool, + use_cache: bool, ) -> Result { debug!( diff_len = diff.len(), @@ -99,6 +115,14 @@ async fn review_diff_inner( }); } + // Check cache before calling LLM + if use_cache { + if let Some(cached) = crate::engine::cache::get_cached_review(diff, config.cache_ttl) { + debug!("returning cached review response"); + return Ok(cached); + } + } + // Extract valid file paths for post-parse filtering let valid_files = llm::extract_file_paths_from_diff(diff); @@ -130,6 +154,13 @@ async fn review_diff_inner( .await? }; + // Save to cache on success + if use_cache { + if let Err(e) = crate::engine::cache::save_cached_review(diff, &response) { + debug!("failed to save review to cache: {}", e); + } + } + // Filter out issues with invalid file paths (hallucination guard) if !valid_files.is_empty() { let before = response.issues.len(); From ab16a4a1a3ddda0a7d5d1df7960e24d0ea2fc9e3 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:28:32 +0700 Subject: [PATCH 07/13] feat: validate git refs to prevent injection and path traversal (#73) - Add validate_ref() function that rejects refs with shell metacharacters - Reject path traversal sequences (../etc/passwd) while allowing valid git range syntax (HEAD~3..HEAD, main...develop) - Apply validation in get_branch_diff() and get_commit_diff() --- src/git/diff.rs | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/src/git/diff.rs b/src/git/diff.rs index 00eb8c29..b0d086bb 100644 --- a/src/git/diff.rs +++ b/src/git/diff.rs @@ -4,6 +4,46 @@ use std::process::Command; use anyhow::{Context, Result}; use git2::Repository; +/// Characters that should not appear in git refs passed to our diff commands. +/// This prevents shell injection and path traversal via crafted ref names. +const DANGEROUS_REF_CHARS: &[char] = &[ + ';', '&', '|', '`', '$', '(', ')', '<', '>', '{', '}', '\\', '\n', '\r', '\t', +]; + +/// Validate a git ref string for safety before passing it to git commands. +/// +/// Rejects refs containing shell metacharacters or path traversal sequences (..). +fn validate_ref(ref_str: &str) -> Result<()> { + if ref_str.contains("..") { + // ".." is only allowed in the middle of range expressions like "HEAD..HEAD" or "main...HEAD" + // But we need to be careful about path traversal. Allow it only for git range syntax. + // Actually, git diff ranges use ".." and "..." which are valid git syntax. + // The risk is when ".." appears as part of a path (e.g., "../../etc/passwd"). + // Since git refs shouldn't contain path components with "..", we reject refs + // that start with ".." or contain "/.." or ".." at the end. + if ref_str.starts_with("..") || ref_str.contains("/..") || ref_str.ends_with("..") { + anyhow::bail!( + "invalid ref '{}': contains path traversal sequence '..'", + ref_str + ); + } + } + + if ref_str.contains(DANGEROUS_REF_CHARS) { + let found: String = ref_str + .chars() + .filter(|c| DANGEROUS_REF_CHARS.contains(c)) + .collect(); + anyhow::bail!( + "invalid ref '{}': contains unsafe characters: '{}'", + ref_str, + found + ); + } + + Ok(()) +} + /// Open the git repository at or above the current working directory. pub fn open_repo() -> Result { Repository::discover(std::env::current_dir()?).context("not inside a git repository") @@ -36,6 +76,7 @@ pub fn get_unstaged_diff() -> Result { /// Get the diff between the current branch and `base_branch`. pub fn get_branch_diff(base_branch: &str) -> Result { + validate_ref(base_branch)?; let arg = format!("{}...HEAD", base_branch); git_cmd(&["diff", &arg]) } @@ -51,6 +92,7 @@ pub fn get_unpushed_diff() -> Result { /// - Otherwise (e.g. `HEAD`, `abc123`), uses `git show --format=""` to /// get just the diff without commit metadata. pub fn get_commit_diff(ref_str: &str) -> Result { + validate_ref(ref_str)?; if ref_str.contains("..") { git_cmd(&["diff", ref_str]) } else { @@ -122,3 +164,46 @@ pub fn is_inside_git_repo(dir: Option<&Path>) -> bool { let search_dir = dir.unwrap_or(&cwd); Repository::discover(search_dir).is_ok() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn validate_ref_simple_name() { + assert!(validate_ref("main").is_ok()); + assert!(validate_ref("HEAD").is_ok()); + assert!(validate_ref("feature/my-branch").is_ok()); + assert!(validate_ref("abc123").is_ok()); + } + + #[test] + fn validate_ref_range_syntax() { + assert!(validate_ref("HEAD~3..HEAD").is_ok()); + assert!(validate_ref("main...develop").is_ok()); + assert!(validate_ref("abc123..def456").is_ok()); + } + + #[test] + fn validate_ref_rejects_path_traversal() { + assert!(validate_ref("../../etc/passwd").is_err()); + assert!(validate_ref("..HEAD").is_err()); + assert!(validate_ref("HEAD..").is_err()); + assert!(validate_ref("branch/../other").is_err()); + } + + #[test] + fn validate_ref_rejects_shell_metacharacters() { + assert!(validate_ref("main;rm -rf /").is_err()); + assert!(validate_ref("main`whoami`").is_err()); + assert!(validate_ref("$(whoami)").is_err()); + assert!(validate_ref("main && echo pwned").is_err()); + assert!(validate_ref("main | cat").is_err()); + assert!(validate_ref("main\nextra").is_err()); + } + + #[test] + fn validate_ref_rejects_angle_brackets() { + assert!(validate_ref("main > /etc/passwd").is_err()); + } +} From 379445aac3582ac4abd34882fc1af9491c99e7fa Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:29:11 +0700 Subject: [PATCH 08/13] feat: add --no-cache flag to review CLI command (#100) - Add no_cache field to ReviewOpts struct - Wire through Command::Review destructuring - Pass to review::ReviewOptions construction --- src/main.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main.rs b/src/main.rs index 5d7f1179..09d1f2aa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -115,6 +115,10 @@ enum Command { #[clap(long, value_parser = ["info", "minor", "major", "critical"])] severity: Option, + /// Disable review caching + #[clap(long)] + no_cache: bool, + /// Upload SARIF output to GitHub Code Scanning after review /// (implies --format sarif) #[clap(long)] @@ -286,6 +290,7 @@ async fn main() -> Result<()> { stream, quiet, severity, + no_cache, upload, repo, ref_name, @@ -303,6 +308,7 @@ async fn main() -> Result<()> { stream, quiet, severity, + no_cache, upload, repo, ref_name, @@ -410,6 +416,7 @@ struct ReviewOpts { repo: Option, ref_name: Option, token: Option, + no_cache: bool, } /// Struct to hold scan options from CLI. @@ -452,6 +459,7 @@ async fn cmd_review(globals: &GlobalOptions, opts: ReviewOpts) -> Result { stream: opts.stream, quiet: opts.quiet, severity: opts.severity.clone(), + no_cache: opts.no_cache, }; // When streaming and not quiet, show a simpler message From dea18b365ac3633582c04cfa0e8a086e7b91724e Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:29:51 +0700 Subject: [PATCH 09/13] fix: save cache after post-processing, not before Cache now stores the fully filtered response (after file path validation and ignore rules), not the raw LLM output. This ensures cache hits return identical results to fresh reviews. --- Cargo.lock | 74 +++++++++++++++++++++++++++++++++++++++++++- src/engine/review.rs | 14 ++++----- 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f84a1475..f9ad90eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -106,6 +106,15 @@ version = "2.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" +[[package]] +name = "block-buffer" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" +dependencies = [ + "generic-array", +] + [[package]] name = "bstr" version = "1.12.1" @@ -231,7 +240,7 @@ dependencies = [ [[package]] name = "cora-cli" -version = "0.1.5" +version = "0.1.6" dependencies = [ "anyhow", "assert_cmd", @@ -249,6 +258,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml_ng", + "sha2", "tempfile", "tokio", "tokio-test", @@ -258,12 +268,41 @@ dependencies = [ "walkdir", ] +[[package]] +name = "cpufeatures" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" +dependencies = [ + "libc", +] + +[[package]] +name = "crypto-common" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" +dependencies = [ + "generic-array", + "typenum", +] + [[package]] name = "difflib" version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" +[[package]] +name = "digest" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" +dependencies = [ + "block-buffer", + "crypto-common", +] + [[package]] name = "dirs" version = "6.0.0" @@ -414,6 +453,16 @@ dependencies = [ "slab", ] +[[package]] +name = "generic-array" +version = "0.14.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" +dependencies = [ + "typenum", + "version_check", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -1357,6 +1406,17 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "sha2" +version = "0.10.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -1740,6 +1800,12 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -1812,6 +1878,12 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "wait-timeout" version = "0.2.1" diff --git a/src/engine/review.rs b/src/engine/review.rs index 2572a058..833421ff 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -154,13 +154,6 @@ async fn review_diff_inner( .await? }; - // Save to cache on success - if use_cache { - if let Err(e) = crate::engine::cache::save_cached_review(diff, &response) { - debug!("failed to save review to cache: {}", e); - } - } - // Filter out issues with invalid file paths (hallucination guard) if !valid_files.is_empty() { let before = response.issues.len(); @@ -195,6 +188,13 @@ async fn review_diff_inner( "review complete" ); + // Save fully-processed response to cache (after filtering) + if use_cache { + if let Err(e) = crate::engine::cache::save_cached_review(diff, &response) { + debug!("failed to save review to cache: {}", e); + } + } + Ok(response) } From d18ef9306c59f50d62452db27d4e8a694c007950 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:33:31 +0700 Subject: [PATCH 10/13] fix: per-request timeout + cache key includes model/temperature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move timeout from Client level to per-request .timeout() on RequestBuilder - Users can now configure llm.timeout and it actually works - Remove misleading dead code (shared_client timeout_secs param) - Cache key now includes model + temperature (config changes invalidate cache) - Add test: different model/temperature produces different cache key - Add test: cache key is deterministic Addresses cora review findings on v0.1.7 diff: - MAJOR: shared client ignores configurable timeout → fixed with per-request timeout - MAJOR: cache key only hashes diff → fixed by including model + temperature --- src/engine/cache.rs | 60 +++++++++++++++++++++++++++++++------------- src/engine/llm.rs | 23 +++++++---------- src/engine/review.rs | 14 +++++++++-- 3 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/engine/cache.rs b/src/engine/cache.rs index d1abf5c0..a1a13d8e 100644 --- a/src/engine/cache.rs +++ b/src/engine/cache.rs @@ -12,9 +12,14 @@ fn cache_dir() -> Result { Ok(home.join(".cache").join("cora").join("reviews")) } -/// Compute SHA-256 hex digest of the diff content. -fn sha256_hex(diff: &str) -> String { - let result = Sha256::digest(diff.as_bytes()); +/// Compute SHA-256 hex digest of the diff content + config parameters. +/// Includes model and temperature so config changes invalidate the cache. +fn cache_key(diff: &str, model: &str, temperature: f32) -> String { + let mut hasher = Sha256::new(); + hasher.update(diff.as_bytes()); + hasher.update(model.as_bytes()); + hasher.update(temperature.to_le_bytes()); + let result = hasher.finalize(); result.iter().map(|b| format!("{:02x}", b)).collect() } @@ -22,8 +27,13 @@ fn sha256_hex(diff: &str) -> String { /// /// Returns `None` if no cache exists or if the cache entry has expired /// (based on `ttl` in minutes). -pub fn get_cached_review(diff: &str, ttl: u64) -> Option { - let hash = sha256_hex(diff); +pub fn get_cached_review( + diff: &str, + model: &str, + temperature: f32, + ttl: u64, +) -> Option { + let hash = cache_key(diff, model, temperature); let dir = cache_dir().ok()?; let path = dir.join(format!("{}.json", hash)); @@ -61,12 +71,17 @@ pub fn get_cached_review(diff: &str, ttl: u64) -> Option { } /// Save a review response to the cache. -pub fn save_cached_review(diff: &str, response: &ReviewResponse) -> Result<()> { +pub fn save_cached_review( + diff: &str, + model: &str, + temperature: f32, + response: &ReviewResponse, +) -> Result<()> { let dir = cache_dir()?; std::fs::create_dir_all(&dir) .with_context(|| format!("failed to create cache dir {}", dir.display()))?; - let hash = sha256_hex(diff); + let hash = cache_key(diff, model, temperature); let path = dir.join(format!("{}.json", hash)); let now = SystemTime::now() @@ -120,24 +135,33 @@ mod tests { } #[test] - fn sha256_hex_is_deterministic() { - let hash1 = sha256_hex("hello world"); - let hash2 = sha256_hex("hello world"); + fn cache_key_is_deterministic() { + let hash1 = cache_key("hello world", "gpt-4", 0.0); + let hash2 = cache_key("hello world", "gpt-4", 0.0); assert_eq!(hash1, hash2); assert_eq!(hash1.len(), 64); // SHA-256 hex = 64 chars } #[test] - fn sha256_hex_differs_for_different_inputs() { - let hash1 = sha256_hex("hello world"); - let hash2 = sha256_hex("hello earth"); + fn cache_key_differs_for_different_inputs() { + let hash1 = cache_key("hello world", "gpt-4", 0.0); + let hash2 = cache_key("hello earth", "gpt-4", 0.0); assert_ne!(hash1, hash2); } #[test] - fn cache_hit_on_same_diff() { + fn cache_key_includes_model_and_temperature() { + let h1 = cache_key("diff", "gpt-4", 0.0); + let h2 = cache_key("diff", "gpt-3.5", 0.0); + let h3 = cache_key("diff", "gpt-4", 0.7); + assert_ne!(h1, h2, "different models should differ"); + assert_ne!(h1, h3, "different temperatures should differ"); + } + + #[test] + fn cache_key_len_is_64() { let diff = "diff --git a/file.txt b/file.txt\n+ hello"; - let hash = sha256_hex(diff); + let hash = cache_key(diff, "model", 0.0); assert_eq!(hash.len(), 64); } @@ -145,8 +169,8 @@ mod tests { fn cache_miss_on_different_diff() { let diff1 = "diff --git a/a.txt b/a.txt\n+ hello"; let diff2 = "diff --git a/b.txt b/b.txt\n+ world"; - let hash1 = sha256_hex(diff1); - let hash2 = sha256_hex(diff2); + let hash1 = cache_key(diff1, "model", 0.0); + let hash2 = cache_key(diff2, "model", 0.0); assert_ne!(hash1, hash2); } @@ -222,7 +246,7 @@ mod tests { // Manually set up a cache entry in the temp dir let diff = "test diff content"; - let hash = sha256_hex(diff); + let hash = cache_key(diff, "model", 0.0); let path = dir.join(format!("{}.json", hash)); let response = make_response(); diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 9adb1786..0a3a6a3e 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -8,28 +8,21 @@ use tracing::debug; use crate::engine::types::{LLMConfig, ReviewIssue, ReviewResponse, TokenUsage}; -/// Shared reqwest::Client with default timeout. Reused across all LLM requests. +/// Shared reqwest::Client with connection pooling. Reused across all LLM requests. /// Created lazily on first use to avoid blocking initialization. +/// Per-request timeout is set via .timeout() on the RequestBuilder. static SHARED_CLIENT: LazyLock = LazyLock::new(|| { reqwest::Client::builder() - .timeout(Duration::from_secs(120)) .pool_max_idle_per_host(4) .build() .unwrap_or_else(|e| { tracing::error!("failed to build shared HTTP client: {}", e); - // Fall back to a default client — this should never happen in practice reqwest::Client::new() }) }); /// Return the shared reqwest::Client for LLM API requests. -/// -/// Note: The timeout is set at client construction time (120s default). -/// Per-request timeout override is not supported by reqwest once the -/// client is built, so the `timeout_secs` parameter is accepted for -/// API consistency but the shared client's built-in timeout is used. -pub fn shared_client(timeout_secs: u64) -> reqwest::Client { - let _ = timeout_secs; // accepted for API consistency, shared client has fixed timeout +pub fn shared_client() -> reqwest::Client { SHARED_CLIENT.clone() } @@ -151,7 +144,7 @@ async fn chat_completion( spinner: Option<&ProgressBar>, response_format: &str, ) -> Result { - let client = shared_client(config.timeout); + let client = shared_client(); let url = format!("{}/chat/completions", config.base_url.trim_end_matches('/')); @@ -190,9 +183,10 @@ async fn chat_completion( .header("Authorization", format!("Bearer {}", config.api_key)) .header("Content-Type", "application/json") .json(&request) + .timeout(std::time::Duration::from_secs(config.timeout)) .send() .await - .context("LLM API request failed")?; + .context("LLM API request failed (or timed out)")?; let status = response.status(); let body = response @@ -345,7 +339,7 @@ async fn chat_completion_stream( use futures_util::StreamExt; use std::io::Write; - let client = shared_client(config.timeout); + let client = shared_client(); let url = format!("{}/chat/completions", config.base_url.trim_end_matches('/')); let mut request_body = serde_json::json!({ @@ -370,9 +364,10 @@ async fn chat_completion_stream( .header("Authorization", format!("Bearer {}", config.api_key)) .header("Content-Type", "application/json") .json(&request_body) + .timeout(std::time::Duration::from_secs(config.timeout)) .send() .await - .context("LLM API streaming request failed")?; + .context("LLM API streaming request failed (or timed out)")?; let status = response.status(); if !status.is_success() { diff --git a/src/engine/review.rs b/src/engine/review.rs index 833421ff..470cf30e 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -117,7 +117,12 @@ async fn review_diff_inner( // Check cache before calling LLM if use_cache { - if let Some(cached) = crate::engine::cache::get_cached_review(diff, config.cache_ttl) { + if let Some(cached) = crate::engine::cache::get_cached_review( + diff, + &llm_config.model, + llm_config.temperature, + config.cache_ttl, + ) { debug!("returning cached review response"); return Ok(cached); } @@ -190,7 +195,12 @@ async fn review_diff_inner( // Save fully-processed response to cache (after filtering) if use_cache { - if let Err(e) = crate::engine::cache::save_cached_review(diff, &response) { + if let Err(e) = crate::engine::cache::save_cached_review( + diff, + &llm_config.model, + llm_config.temperature, + &response, + ) { debug!("failed to save review to cache: {}", e); } } From bca318f8aa90678897c30cff5badb945a86a6535 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:34:44 +0700 Subject: [PATCH 11/13] chore: remove unused Duration import --- src/engine/llm.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 0a3a6a3e..9726732a 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -3,7 +3,6 @@ use indicatif::{ProgressBar, ProgressStyle}; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::sync::LazyLock; -use std::time::Duration; use tracing::debug; use crate::engine::types::{LLMConfig, ReviewIssue, ReviewResponse, TokenUsage}; From f78420c9b78ffa400bfa98d4d6995090618ef3c9 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:35:31 +0700 Subject: [PATCH 12/13] fix: resilient version resolution in composite actions (#102) - curl -sfL (fail on HTTP errors, silent progress) - 3 retries with 5s delay on failure - python3 .get() instead of direct key access (no KeyError) - Fallback to v0.1.6 with ::warning:: if all retries fail - Fixed in both cora-review and cora-review-simple actions --- .github/actions/cora-review-simple/action.yml | 17 ++++++++++++++++- .github/actions/cora-review/action.yml | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/.github/actions/cora-review-simple/action.yml b/.github/actions/cora-review-simple/action.yml index ba7808ac..cbaf77ef 100644 --- a/.github/actions/cora-review-simple/action.yml +++ b/.github/actions/cora-review-simple/action.yml @@ -39,7 +39,22 @@ runs: shell: bash run: | if [ "${{ inputs.cora-version }}" = "latest" ]; then - VERSION=$(curl -sL https://api.github.com/repos/ajianaz/cora-cli/releases/latest | python3 -c "import sys,json; print(json.load(sys.stdin)['tag_name'])") + # Try up to 3 times with delay + for i in 1 2 3; do + VERSION=$(curl -sfL --connect-timeout 10 \ + https://api.github.com/repos/ajianaz/cora-cli/releases/latest \ + | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('tag_name', ''))" 2>/dev/null) + if [ -n "$VERSION" ]; then + break + fi + echo "Attempt $i failed, retrying..." + sleep 5 + done + # Fallback to a known good version if all retries fail + if [ -z "$VERSION" ]; then + echo "::warning::Failed to resolve latest cora-cli version from GitHub API, using fallback" + VERSION="v0.1.6" + fi else VERSION="${{ inputs.cora-version }}" fi diff --git a/.github/actions/cora-review/action.yml b/.github/actions/cora-review/action.yml index cc3ba451..508891aa 100644 --- a/.github/actions/cora-review/action.yml +++ b/.github/actions/cora-review/action.yml @@ -54,7 +54,22 @@ runs: shell: bash run: | if [ "${{ inputs.cora-version }}" = "latest" ]; then - VERSION=$(curl -sL https://api.github.com/repos/ajianaz/cora-cli/releases/latest | python3 -c "import sys,json; print(json.load(sys.stdin)['tag_name'])") + # Try up to 3 times with delay + for i in 1 2 3; do + VERSION=$(curl -sfL --connect-timeout 10 \ + https://api.github.com/repos/ajianaz/cora-cli/releases/latest \ + | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('tag_name', ''))" 2>/dev/null) + if [ -n "$VERSION" ]; then + break + fi + echo "Attempt $i failed, retrying..." + sleep 5 + done + # Fallback to a known good version if all retries fail + if [ -z "$VERSION" ]; then + echo "::warning::Failed to resolve latest cora-cli version from GitHub API, using fallback" + VERSION="v0.1.6" + fi else VERSION="${{ inputs.cora-version }}" fi From 5e9c39db273aea97bf8f345c411cf8bda1837dfc Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 15:36:18 +0700 Subject: [PATCH 13/13] release: v0.1.7 Fixed non-deterministic reviews (temperature=0), HTTP timeout, caching, git ref validation, config error handling, composite action resilience. Closes #97 #98 #99 #100 #101 #73 #78 #102 --- CHANGELOG.md | 18 ++++++++++++++++++ Cargo.toml | 2 +- README.md | 8 ++++---- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe2b8790..cf6a8a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.1.7] - 2026-06-01 + +### Added + +- **Diff-hash caching** — review results cached by SHA-256 of diff + model + temperature in `~/.cache/cora/reviews/`. Cache TTL configurable via `llm.cache_ttl` (#100) +- **`--no-cache` flag** — bypass cache for fresh reviews (#100) +- **Configurable LLM parameters** — `llm.temperature` (default: 0), `llm.max_tokens` (default: 4096), `llm.timeout` (default: 120s), `llm.cache_ttl` (default: 1440 min) in `.cora.yaml` (#98 #101) +- **Git ref validation** — rejects refs containing shell metacharacters or path traversal sequences (#73) + +### Fixed + +- **Temperature default now 0** — eliminates non-deterministic LLM output. Same diff produces identical issues on every run (#98, #97) +- **HTTP timeout actually works** — per-request timeout via reqwest RequestBuilder (not client-level). Configurable timeout respected (#99) +- **Connection pooling** — shared reqwest::Client via LazyLock, reused across all requests (#99) +- **Cache key includes model + temperature** — config changes invalidate cache automatically (#100) +- **Silent config corruption** — malformed `.cora.yaml` now shows clear error with file path and hint (#78) +- **Composite action KeyError on API failure** — version resolution retries 3x with 5s delay, falls back to v0.1.6 with warning. Fixed in both `cora-review` and `cora-review-simple` actions (#102) + ## [0.1.6] - 2026-06-01 ### Added diff --git a/Cargo.toml b/Cargo.toml index cb28f892..11f82b31 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cora-cli" -version = "0.1.6" +version = "0.1.7" edition = "2024" description = "CLI-first AI code review — BYOK, diff/scan/branch, pre-commit hooks" license = "MIT" diff --git a/README.md b/README.md index 28133559..8c605b16 100644 --- a/README.md +++ b/README.md @@ -41,10 +41,10 @@ Download the latest release from [GitHub Releases](https://github.com/ajianaz/co ```bash # Determine your platform tag from the releases page, e.g.: -# cora-aarch64-unknown-linux-gnu-v0.1.6.tar.gz -# cora-x86_64-unknown-linux-gnu-v0.1.6.tar.gz -# cora-aarch64-apple-darwin-v0.1.6.tar.gz -# cora-x86_64-pc-windows-msvc-v0.1.6.zip +# cora-aarch64-unknown-linux-gnu-v0.1.7.tar.gz +# cora-x86_64-unknown-linux-gnu-v0.1.7.tar.gz +# cora-aarch64-apple-darwin-v0.1.7.tar.gz +# cora-x86_64-pc-windows-msvc-v0.1.7.zip # Example: Linux aarch64 VERSION=$(curl -s https://api.github.com/repos/ajianaz/cora-cli/releases/latest | grep tag_name | cut -d'"' -f4)