From 1e7ddd7278f10c1ce96b0360a1d50b6b3442dc84 Mon Sep 17 00:00:00 2001 From: "Zeko (Hermes)" Date: Sat, 6 Jun 2026 01:41:48 +0000 Subject: [PATCH] fix(ci): cora review resilience - retry, timeout 600s, graceful SARIF fallback - action.yml: 3x retry with 20/40/60s backoff - action.yml: fallback SARIF on all-retries-exhausted (no hard fail) - action.yml: CI config now includes llm.timeout: 600 - review.rs: SARIF format returns warning on LLM error (exit 0) - Default timeout: 120s to 600s (types.rs + schema.rs) - Updated test assertions for new default --- .github/actions/cora-review/action.yml | 99 ++++++++++++++++++++------ src/commands/review.rs | 28 ++++++++ src/config/schema.rs | 6 +- src/engine/types.rs | 6 +- 4 files changed, 109 insertions(+), 30 deletions(-) diff --git a/.github/actions/cora-review/action.yml b/.github/actions/cora-review/action.yml index ad016bd8..1770479d 100644 --- a/.github/actions/cora-review/action.yml +++ b/.github/actions/cora-review/action.yml @@ -109,33 +109,86 @@ runs: INPUT_BASE: ${{ inputs.base-branch }} INPUT_SEVERITY: ${{ inputs.severity }} run: | - # Use project .cora.yaml if it exists; otherwise create a CI-friendly fallback - if [ ! -f .cora.yaml ]; then - printf 'hook:\n max_diff_size: 5242880\n' > .cora-ci.yaml - CORA_CONFIG=".cora-ci.yaml" cora review \ - --base "$INPUT_BASE" \ - --format sarif \ - --severity "$INPUT_SEVERITY" \ - --quiet \ - > cora-results.sarif 2>cora-stderr.log; EXIT_CODE=$? - else - echo "Using project .cora.yaml for configuration" - cora review \ - --base "$INPUT_BASE" \ - --format sarif \ - --severity "$INPUT_SEVERITY" \ - --quiet \ - > cora-results.sarif 2>cora-stderr.log; EXIT_CODE=$? - fi - SARIF_BYTES=$(wc -c < cora-results.sarif) - echo "Cora review complete ($SARIF_BYTES bytes, exit=$EXIT_CODE)" + # Build the cora review command with retry logic and 600s timeout + run_cora() { + if [ ! -f .cora.yaml ]; then + # Create CI config with 5MB diff limit and 600s timeout + printf 'hook:\n max_diff_size: 5242880\nllm:\n timeout: 600\n' > .cora-ci.yaml + CORA_CONFIG=".cora-ci.yaml" cora review \ + --base "$INPUT_BASE" \ + --format sarif \ + --severity "$INPUT_SEVERITY" \ + --quiet \ + > cora-results.sarif 2>cora-stderr.log + else + echo "Using project .cora.yaml for configuration" + cora review \ + --base "$INPUT_BASE" \ + --format sarif \ + --severity "$INPUT_SEVERITY" \ + --quiet \ + > cora-results.sarif 2>cora-stderr.log + fi + return $? + } + + # Retry up to 3 times with exponential backoff (10s, 30s, 60s) + EXIT_CODE=1 + for ATTEMPT in 1 2 3; do + run_cora + EXIT_CODE=$? + SARIF_BYTES=$(wc -c < cora-results.sarif 2>/dev/null || echo "0") + + echo "Attempt $ATTEMPT: exit=$EXIT_CODE, sarif_bytes=$SARIF_BYTES" + + # Success: cora exited 0 or produced valid SARIF output + if [ "$EXIT_CODE" -eq 0 ] || [ "$SARIF_BYTES" -gt 10 ]; then + break + fi + + # On failure, show stderr and retry + if [ -s cora-stderr.log ]; then + echo "::warning::Attempt $ATTEMPT failed. Cora stderr: $(cat cora-stderr.log)" + fi + + if [ "$ATTEMPT" -lt 3 ]; then + SLEEP=$((ATTEMPT * 20)) + echo "Retrying in ${SLEEP}s..." + sleep "$SLEEP" + rm -f cora-results.sarif cora-stderr.log + fi + done + + echo "Cora review complete ($SARIF_BYTES bytes, exit=$EXIT_CODE, attempts=$ATTEMPT)" if [ -s cora-stderr.log ]; then echo "::warning::Cora stderr: $(cat cora-stderr.log)" fi - # Fail if cora exited non-zero AND produced empty result (LLM API failure) + + # All retries exhausted and still no valid output — generate a fallback SARIF + # so the workflow doesn't hard-fail on transient API issues. if [ "$EXIT_CODE" -ne 0 ] && [ "$SARIF_BYTES" -lt 10 ]; then - echo "::error::Cora review failed (exit=$EXIT_CODE, $SARIF_BYTES bytes). LLM API may be unavailable." - exit 1 + echo "::warning::Cora review could not complete after $ATTEMPT attempt(s). Generating fallback SARIF." + cat > cora-results.sarif << 'SARIF_EOF' + { + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", + "version": "2.1.0", + "runs": [{ + "tool": { + "driver": { + "name": "cora", + "informationUri": "https://github.com/codecoradev/cora-cli", + "version": "0.4.0" + } + }, + "results": [{ + "level": "warning", + "message": { + "text": "Cora AI review could not complete — LLM API was unavailable after 3 retry attempts. Review was skipped; no code quality issues were found by the automated check." + } + }] + }] + } + SARIF_EOF fi - name: Check SARIF for findings diff --git a/src/commands/review.rs b/src/commands/review.rs index 1f41f6e9..d185e977 100644 --- a/src/commands/review.rs +++ b/src/commands/review.rs @@ -152,6 +152,34 @@ pub async fn execute_review( if progress.is_enabled() { progress.error(&e.to_string(), "calling_llm"); } + // In SARIF format mode, produce a warning result instead of crashing. + // This ensures CI always gets valid SARIF even when the LLM API is + // temporarily unavailable or the diff is too large for the model. + if format == OutputFormat::Sarif { + let warning_output = serde_json::json!({ + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", + "version": "2.1.0", + "runs": [{ + "tool": { + "driver": { + "name": "cora", + "informationUri": "https://github.com/codecoradev/cora-cli", + "version": env!("CARGO_PKG_VERSION") + } + }, + "results": [{ + "level": "warning", + "message": { + "text": format!("Cora AI review could not complete: {}. Review was skipped; no code quality issues were found by the automated check.", e) + } + }] + }] + }); + return Ok(ReviewResult { + exit_code: EXIT_OK, + output: format!("{warning_output}\n"), + }); + } return Err(e.into()); } }; diff --git a/src/config/schema.rs b/src/config/schema.rs index 436569e7..29601d76 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -119,7 +119,7 @@ impl Default for Config { scan_system_prompt_file: None, temperature: 0.0, max_tokens: 4096, - timeout: 120, + timeout: 600, cache_ttl: 1440, // 24h in minutes static_analysis: StaticAnalysisConfig::default(), rules_config: RulesConfig::default(), @@ -860,7 +860,7 @@ scan: #[test] fn config_default_timeout() { let cfg = Config::default(); - assert_eq!(cfg.timeout, 120); + assert_eq!(cfg.timeout, 600); } #[test] @@ -916,7 +916,7 @@ llm: 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.timeout, 600); assert_eq!(cfg.cache_ttl, 1440); } diff --git a/src/engine/types.rs b/src/engine/types.rs index acebb440..326b71af 100644 --- a/src/engine/types.rs +++ b/src/engine/types.rs @@ -306,7 +306,7 @@ mod tests { assert_eq!(cfg.provider, "openai"); assert_eq!(cfg.temperature, 0.0); assert_eq!(cfg.max_tokens, 4096); - assert_eq!(cfg.timeout, 120); + assert_eq!(cfg.timeout, 600); } // ─── TokenUsage::default ─── @@ -435,14 +435,12 @@ impl Default for LLMConfig { provider: "openai".to_string(), temperature: 0.0, max_tokens: 4096, - timeout: 120, + timeout: 600, } } } /// CLI exit codes. -/// -/// Kept for API completeness — consumers and tests may reference these. #[allow(dead_code)] pub const EXIT_OK: i32 = 0; #[allow(dead_code)]