AI review: give reviewers the failing step of each CI job, ignore Trivy failures

ai-review/ai_review_gate.sh

@@ -11,7 +11,7 @@
 # if purchased from OpenC3, Inc.
 
 # Decides whether the AI review loop should run for a PR and collects failed
-# CI job logs and SonarQube findings for the reviewers. Every CI workflow completion triggers the AI
+# CI job logs (but not Trivy scan failures) and SonarQube findings for the reviewers. Every CI workflow completion triggers the AI
 # Review workflow, so this lets only the run that sees all CI finished proceed.
 # The Malicious Code Scan does not trigger the review, so that run waits for a
 # scan still running on the PR head. A manual (workflow_dispatch) run fails
@@ -157,31 +157,61 @@ case "$scan_state" in
 esac
 
 # Collect failed job logs, with a workflow-level fallback for failures before jobs start.
+log_file="$(mktemp)"
+log_error="$(mktemp)"
+trap 'rm -f "$log_file" "$log_error"' EXIT
+# A job log ends with its post steps (harden-runner's cleanup alone runs to well over 100 lines),
+# so keep the last LOG_LINES before the last error ahead of them: the end of the step that failed.
+# Strip the timestamp prefix and ANSI colors, and cut very long lines, to save tokens.
+failed_step_log() {
+  sed -E 's/^\xef\xbb\xbf//; s/^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9:.]+Z //; s/\x1b\[[0-9;]*m//g' \
+    | awk -v n="$LOG_LINES" '
+      /^Post job cleanup\.$/ { exit }
+      { line[NR] = $0; if (/^##\[error\]/) last = NR }
+      END {
+        stop = last ? last : NR
+        for (i = (stop > n ? stop - n + 1 : 1); i <= stop; i++) print substr(line[i], 1, 1000)
+      }'
+}
 failures=0
 while IFS=$'\t' read -r run_id run_name run_conclusion run_url; do
   [[ -n "$run_id" ]] || continue
   failures_before=$failures
+  ignored=0
+  # A job whose only failed steps are Trivy scans failed on a vulnerability in a base image or
+  # dependency, which a fix to the PR cannot address, so the reviewers never see it
+  # shellcheck disable=SC2016
   jobs="$(gh api "repos/$repo/actions/runs/$run_id/jobs" --paginate \
-    --jq '.jobs[] | select(.conclusion == "failure" or .conclusion == "timed_out") | [.id, .name, .html_url] | @tsv')" \
+    --jq '.jobs[] | select(.conclusion == "failure" or .conclusion == "timed_out")
+      | [.steps[]? | select(.conclusion == "failure" or .conclusion == "timed_out") | .name] as $failed
+      | [.id, .name, .html_url, ($failed != [] and all($failed[]; test("trivy"; "i")))] | @tsv')" \
     || jobs=""
-  while IFS=$'\t' read -r job_id job_name job_url; do
+  while IFS=$'\t' read -r job_id job_name job_url trivy_only; do
     [[ -n "$job_id" ]] || continue
+    if [[ "$trivy_only" == "true" ]]; then
+      echo "::notice::Ignoring $run_name / $job_name: only its Trivy scan failed"
+      ignored=$((ignored + 1))
+      continue
+    fi
     failures=$((failures + 1))
     {
       echo "### $run_name / $job_name"
       echo
       echo "$job_url"
       echo
       echo '```'
-      # Strip the timestamp prefix and ANSI colors to save tokens
-      gh api "repos/$repo/actions/jobs/$job_id/logs" 2> /dev/null \
-        | sed -E 's/^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9:.]+Z //; s/\x1b\[[0-9;]*m//g' \
-        | tail -n "$LOG_LINES" || echo "(log unavailable)"
+      if gh api "repos/$repo/actions/jobs/$job_id/logs" > "$log_file" 2> "$log_error"; then
+        failed_step_log < "$log_file"
+      else
+        error="$(tr -s '\n' ' ' < "$log_error" | sed 's/ *$//' | cut -c1-300)"
+        echo "::warning::Could not download the log of $run_name / $job_name: ${error:-unknown error}" >&2
+        echo "(log unavailable: ${error:-unknown error})"
+      fi
       echo '```'
       echo
     } >> "$CI_FILE"
   done <<< "$jobs"
-  if (( failures == failures_before )); then
+  if (( failures == failures_before && ignored == 0 )); then
     failures=$((failures + 1))
     {
       echo "### $run_name / workflow failure"

tests/test_ai_review.py

@@ -99,6 +99,7 @@ def workflow_script(name, workflow=WORKFLOW):
     value = sequence.pop(0) if len(sequence) > 1 else sequence[0]
     fixture_path.write_text(json.dumps(fixtures))
 if isinstance(value, dict) and value.get('test_api_error'):
+    print('test API error', file=sys.stderr)
     sys.exit(1)
 if isinstance(value, dict) and 'test_zip' in value:
     buffer = io.BytesIO()
@@ -1011,6 +1012,71 @@ def test_failed_job_logs_are_not_double_counted(self):
         self.assertIn("An actual lint failure", report)
         self.assertNotIn("workflow failure", report)
 
+    def test_failed_job_log_keeps_the_failed_step_not_the_post_steps(self):
+        self.fixtures["repos/owner/repo/actions/runs/123/jobs"] = {
+            "jobs": [{"id": 10, "name": "unit", "conclusion": "failure", "html_url": "https://example.invalid/job/10"}]
+        }
+        cleanup = "\n".join(f"2026-01-01T00:00:01.0000000Z cleanup noise {i}" for i in range(300))
+        self.fixtures["repos/owner/repo/actions/jobs/10/logs"] = (
+            "\ufeff2026-01-01T00:00:00.0000000Z \x1b[31mFailures:\x1b[0m\n"
+            "2026-01-01T00:00:00.0000000Z   1) expected 1, got 2\n"
+            "2026-01-01T00:00:00.0000000Z ##[error]Process completed with exit code 1.\n"
+            "2026-01-01T00:00:00.0000000Z Post job cleanup.\n" + cleanup
+        )
+        result = self.run_shell(f'bash "{GATE}"')
+        self.assertEqual(result.returncode, 0, result.stderr)
+        report = (self.directory / "out/ci_failures.md").read_text()
+        self.assertIn("\nFailures:\n  1) expected 1, got 2\n##[error]Process completed", report)
+        self.assertNotIn("cleanup noise", report)
+        self.assertNotIn("2026-01-01T", report)
+
+    def test_failed_job_log_download_error_is_reported(self):
+        self.fixtures["repos/owner/repo/actions/runs/123/jobs"] = {
+            "jobs": [{"id": 10, "name": "unit", "conclusion": "failure", "html_url": "https://example.invalid/job/10"}]
+        }
+        self.fixtures["repos/owner/repo/actions/jobs/10/logs"] = {"test_api_error": True}
+        result = self.run_shell(f'bash "{GATE}"')
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(self.outputs()["ci_failures"], "1")
+        self.assertIn("::warning::Could not download the log of", result.stderr)
+        self.assertIn("(log unavailable: test API error)", (self.directory / "out/ci_failures.md").read_text())
+
+    def test_trivy_scan_failures_are_ignored(self):
+        def job(job_id, *failed_steps):
+            steps = [{"name": "Build", "conclusion": "success"}]
+            steps += [{"name": name, "conclusion": "failure"} for name in failed_steps]
+            return {
+                "id": job_id,
+                "name": f"job {job_id}",
+                "conclusion": "failure",
+                "html_url": f"https://example.invalid/job/{job_id}",
+                "steps": steps,
+            }
+
+        self.fixtures["repos/owner/repo/actions/runs/123/jobs"] = {
+            "jobs": [job(10, "Run Trivy on image ruby", "Run trivy on image node")]
+        }
+        result = self.run_shell(f'bash "{GATE}"')
+        self.assertEqual(result.returncode, 0, result.stderr)
+        # The head is an AI review fix, so a scan failure alone does not send it back for another round
+        self.assertEqual(self.outputs()["skip"], "true")
+        self.assertIn("CI passed", self.outputs()["reason"])
+        self.assertEqual((self.directory / "out/ci_failures.md").read_text(), "")
+        self.assertIn("Ignoring Python Lint / job 10", result.stdout)
+
+        # A failure besides the scan still reaches the reviewers
+        self.fixtures["repos/owner/repo/actions/runs/123/jobs"] = {
+            "jobs": [job(10, "Run Trivy on image ruby"), job(11, "Run Trivy on image ruby", "Build images")]
+        }
+        self.fixtures["repos/owner/repo/actions/jobs/11/logs"] = "The build broke"
+        self.outputs_path.unlink()
+        result = self.run_shell(f'bash "{GATE}"')
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertEqual(self.outputs()["ci_failures"], "1")
+        report = (self.directory / "out/ci_failures.md").read_text()
+        self.assertIn("The build broke", report)
+        self.assertNotIn("job 10", report)
+
     def test_pending_builtin_run_does_not_block_review(self):
         runs = self.fixtures["repos/owner/repo/actions/runs?head_sha=test-head&per_page=100"]["workflow_runs"]
         runs.append({"id": 124, "name": "CodeQL", "event": "dynamic", "status": "in_progress", "conclusion": None})
@@ -1205,7 +1271,7 @@ def test_ai_fix_commit_goes_again_for_sonarqube_findings(self):
         self.outputs_path.unlink()
         result = self.run_shell(f'bash "{GATE}"')
         self.assertEqual(result.returncode, 0, result.stderr)
-        self.assertEqual(self.outputs()["skip"], "false")
+        self.assertEqual(self.outputs()["skip"], "false", self.outputs().get("reason"))
         self.assertEqual(self.outputs()["ci_failures"], "0")
 
     def test_loop_hands_sonarqube_findings_to_the_reviewers(self):