dc668958e7f016ce98df5c4b16dbab1598634950 lrnassar Thu Jul 9 12:53:40 2026 -0700 Add failure alerting to codeReviewAi.py daily mode and make output dir userless. refs #36890 Detect Claude CLI hard failures (auth/401 errors, empty output, or output that fails validation) instead of silently saving a broken review and marking it APPROVED. On failure the review is never emailed to the author; instead a single consolidated alert is emailed to browserqa-group@ucsc.edu and the script exits non-zero so the cron's failure guard fires. This addresses the daily reviews having failed silently since a token expiry. Replace the hardcoded /hive/users/lrnassar/codeReview output path with one derived from the current user (getpass.getuser()), and create it if missing so ticket/commit modes work for any account. diff --git src/utils/codeReviewAi.py src/utils/codeReviewAi.py index 19fba5ade69..30467301805 100755 --- src/utils/codeReviewAi.py +++ src/utils/codeReviewAi.py @@ -9,44 +9,46 @@ Daily mode: Use --daily to review all commits from the last N hours, bundled by author, and email each author with the review (designed to run as a daily cron) Usage: python3 codeReviewAi.py [--dry-run] [--ticket TICKET_ID] python3 codeReviewAi.py --ticket TICKET_ID --commit COMMIT_HASH [--dry-run] python3 codeReviewAi.py --commit COMMIT_HASH [--dry-run] python3 codeReviewAi.py --daily [--hours 24] [--cc list@example.com] [--dry-run] """ import os import sys import re import json import base64 +import getpass import subprocess import argparse import requests from datetime import datetime, timedelta from email.mime.text import MIMEText from collections import defaultdict # Configuration REDMINE_URL = "https://redmine.gi.ucsc.edu" GIT_REPORTS_PATH = "/hive/groups/qa/git-reports-history" GIT_REPO_PATH = "/data/git/kent.git" -OUTPUT_DIR = "/hive/users/lrnassar/codeReview" +OUTPUT_DIR = f"/hive/users/{getpass.getuser()}/codeReview" MLQ_CONF_PATH = os.path.expanduser("~/.hg.conf") DEFAULT_CC = "browser-code-reviews-group@ucsc.edu" +DEFAULT_ALERT_EMAIL = "browserqa-group@ucsc.edu" GMAIL_TOKEN_PATH = os.path.expanduser("~/.gmail_token.json") GMAIL_CREDS_PATH = os.path.expanduser("~/.gmail_credentials.json") GMAIL_SCOPES = [ 'https://www.googleapis.com/auth/gmail.send', ] CLAUDE_CLI = os.path.expanduser('~/.local/bin/claude') def load_config(): """Load API keys from ~/.hg.conf""" config = {} if not os.path.exists(MLQ_CONF_PATH): print(f"ERROR: Config file not found: {MLQ_CONF_PATH}") sys.exit(1) with open(MLQ_CONF_PATH, 'r') as f: @@ -450,30 +452,55 @@ """Check if the response contains a valid daily review in plain text format""" if not response: return False, "Empty response" if 'DAILY CODE REVIEW' not in response: return False, "Missing DAILY CODE REVIEW header" if 'APPROVED' not in response and 'FEEDBACK' not in response: return False, "Missing verdict section" if len(response) < 500: return False, f"Response too short ({len(response)} chars) - may be incomplete" return True, "OK" +# Substrings that indicate the Claude CLI could not authenticate or otherwise +# failed to produce a usable review. Used to alert instead of failing silently. +CLI_AUTH_ERROR_MARKERS = ( + 'Failed to authenticate', + 'authentication_error', + 'Invalid authentication credentials', + 'API Error: 401', +) + +def detect_cli_failure(response, validator): + """Return an error description if the CLI response indicates a hard failure + (no output, an authentication error, or output that fails validation), + otherwise return None. Lets the caller alert rather than silently save a + broken review.""" + if not response: + return "No response from Claude CLI (timeout, crash, or empty output)" + for marker in CLI_AUTH_ERROR_MARKERS: + if marker in response: + first_line = next((l for l in response.strip().splitlines() if l.strip()), response) + return f"Claude CLI authentication failure: {first_line.strip()[:300]}" + is_valid, msg = validator(response) + if not is_valid: + return f"Invalid or incomplete review output: {msg}" + return None + def call_claude_cli(prompt, timeout=600, retries=1, validator=None): """Call Claude Code CLI with a prompt and return the response""" if validator is None: validator = validate_review_output for attempt in range(retries + 1): try: result = subprocess.run( [CLAUDE_CLI, '-p', prompt, '--output-format', 'text', '--allowedTools', 'Bash,Read,Glob,Grep,Agent'], capture_output=True, text=True, timeout=timeout ) if result.returncode != 0: @@ -1238,156 +1265,230 @@ """Send a code review email to the author""" message = MIMEText(review_text) message['To'] = to_email message['From'] = 'gbauto@ucsc.edu' message['Subject'] = f'Daily Code Review - {author_name} - {datetime.now().strftime("%Y-%m-%d")}' if cc: message['Cc'] = cc encoded = base64.urlsafe_b64encode(message.as_bytes()).decode('utf-8') gmail_service.users().messages().send( userId='me', body={'raw': encoded} ).execute() +def send_alert_email(gmail_service, to_email, failures, hours, log_dir): + """Send a maintainer alert when one or more author reviews failed (e.g. an + expired Claude CLI token). This is what keeps the daily cron from failing + silently: a broken review is never sent to the author, so without this + nobody would notice the tool had stopped working.""" + lines = [ + "The automated daily code review (codeReviewAi.py --daily) hit errors and", + "did not produce valid reviews for one or more authors.", + "", + f"Review date: {datetime.now().strftime('%Y-%m-%d %H:%M')}", + f"Look-back window: {hours} hours", + f"Failed reviews: {len(failures)}", + "", + "Details:", + ] + for name, err in failures: + lines.append(f" - {name}: {err}") + lines += [ + "", + "Most common cause: the 'claude' CLI OAuth token for the cron user has", + "expired. Re-run 'claude' interactively as that user and /login, or mint a", + "long-lived token with 'claude setup-token', then confirm with:", + " claude -p 'say ok'", + "", + f"Logs: {log_dir}/", + "", + "-- codeReviewAi.py automated alert", + ] + message = MIMEText("\n".join(lines)) + message['To'] = to_email + message['From'] = 'gbauto@ucsc.edu' + message['Subject'] = f"[ALERT] Daily Code Review FAILED - {datetime.now().strftime('%Y-%m-%d')}" + encoded = base64.urlsafe_b64encode(message.as_bytes()).decode('utf-8') + gmail_service.users().messages().send( + userId='me', + body={'raw': encoded} + ).execute() + + def review_daily_author(author_name, commits, log_dir): """Review all commits by one author for the daily digest. Temp files (prompts/responses) are written to log_dir and returned for cleanup.""" print(f"\n{'='*60}") print(f"REVIEWING DAILY COMMITS: {author_name}") print(f"Commits: {len(commits)}") print(f"{'='*60}") prompt = build_daily_review_prompt(author_name, commits) # Save prompt to log_dir for debugging (cleaned up on success) safe_name = re.sub(r'[^a-zA-Z0-9]', '_', author_name) date_str = datetime.now().strftime('%Y%m%d') temp_files = [] prompt_file = os.path.join(log_dir, f".tmp_daily_prompt_{safe_name}_{date_str}.txt") with open(prompt_file, 'w') as f: f.write(prompt) temp_files.append(prompt_file) print(f" Prompt saved to: {prompt_file}") print(f" Calling Claude CLI (this may take a few minutes)...") - response = call_claude_cli(prompt, timeout=600, validator=validate_daily_review_output) + raw_response = call_claude_cli(prompt, timeout=600, validator=validate_daily_review_output) + error = detect_cli_failure(raw_response, validate_daily_review_output) - if response: + # Save whatever we got back for debugging, even on failure. + if raw_response: response_file = os.path.join(log_dir, f".tmp_daily_response_{safe_name}_{date_str}.txt") with open(response_file, 'w') as f: - f.write(response) + f.write(raw_response) temp_files.append(response_file) + + if error: + print(f" WARNING: review failed - {error}") + response = f"DAILY CODE REVIEW - {author_name}\n\nError: {error}\n" + else: + response = raw_response # Strip any preamble before "DAILY CODE REVIEW" match = re.search(r'^DAILY CODE REVIEW', response, re.MULTILINE) if match: response = response[match.start():] print(f" Review complete") - else: - print(f" WARNING: No response received") - response = f"DAILY CODE REVIEW - {author_name}\n\nError: Review generation failed - no response from Claude CLI.\n" - return response, temp_files + return response, temp_files, error -def run_daily_mode(hours, cc_address, dry_run, log_dir): - """Run daily review mode: get recent commits, review per author, email results""" +def run_daily_mode(hours, cc_address, dry_run, log_dir, alert_email=DEFAULT_ALERT_EMAIL): + """Run daily review mode: get recent commits, review per author, email results. + Returns True on full success, False if any author's review failed (so the + caller can exit non-zero).""" os.makedirs(log_dir, exist_ok=True) print("=" * 60) print(f"DAILY CODE REVIEW MODE") print(f"Looking back: {hours} hours") print(f"CC: {cc_address or 'None'}") + print(f"Alert: {alert_email or 'None'}") print(f"Log dir: {log_dir}") print(f"Dry run: {dry_run}") print("=" * 60) # Phase 1: Gather commits print(f"\nPhase 1: Gathering commits from the last {hours} hours...") authors = get_commits_since(hours) if not authors: print("No commits found in the specified time window.") return total_commits = sum(len(a['commits']) for a in authors.values()) print(f"Found {total_commits} commit(s) from {len(authors)} author(s):") for email, data in authors.items(): print(f" {data['name']} <{email}>: {len(data['commits'])} commit(s)") # Phase 2: Review each author's commits print(f"\nPhase 2: Reviewing commits...") reviews = {} all_temp_files = [] for author_email, data in authors.items(): - review, temp_files = review_daily_author(data['name'], data['commits'], log_dir) + review, temp_files, error = review_daily_author(data['name'], data['commits'], log_dir) all_temp_files.extend(temp_files) reviews[author_email] = { 'name': data['name'], 'email': author_email, 'review': review, 'num_commits': len(data['commits']), + 'error': error, } # Save review to log_dir safe_name = re.sub(r'[^a-zA-Z0-9]', '_', data['name']) date_str = datetime.now().strftime('%Y%m%d') filepath = os.path.join(log_dir, f"daily_review_{safe_name}_{date_str}.txt") with open(filepath, 'w') as f: f.write(review) reviews[author_email]['file'] = filepath print(f" Saved: {filepath}") - # Phase 3: Send emails (only for reviews with FEEDBACK) + # Collect any failures (broken/auth-failed reviews). A failed review is + # never emailed to an author; instead we alert the maintainer below. + failures = [(d['name'], d['error']) for d in reviews.values() if d['error']] + + def verdict_of(data): + if data['error']: + return 'FAILED' + return 'FEEDBACK' if 'OVERALL STATUS: FEEDBACK' in data['review'] else 'APPROVED' + + # Phase 3: Send emails (only for reviews with FEEDBACK; never for failures) print(f"\nPhase 3: Sending emails (FEEDBACK only)...") if dry_run: print("[DRY RUN] Emails not sent. Reviews saved locally:") for author_email, data in reviews.items(): - verdict = 'FEEDBACK' if 'OVERALL STATUS: FEEDBACK' in data['review'] else 'APPROVED' - print(f" {data['name']} <{author_email}>: {verdict} - {data['file']}") + print(f" {data['name']} <{author_email}>: {verdict_of(data)} - {data['file']}") else: gmail_service = get_gmail_service() for author_email, data in reviews.items(): - has_feedback = 'OVERALL STATUS: FEEDBACK' in data['review'] - if not has_feedback: + if data['error']: + print(f" {data['name']}: FAILED - skipping author email (will alert maintainer)") + continue + if 'OVERALL STATUS: FEEDBACK' not in data['review']: print(f" {data['name']}: APPROVED - skipping email") continue print(f" Emailing {data['name']} <{author_email}> (FEEDBACK)...") try: send_review_email(gmail_service, author_email, data['name'], data['review'], cc=cc_address) print(f" SENT") except Exception as e: print(f" FAILED: {e}") + # Phase 3b: Alert the maintainer if anything failed, so it does not fail silently + if failures: + print(f"\nPhase 3b: {len(failures)} review(s) FAILED - alerting maintainer...") + for name, err in failures: + print(f" {name}: {err}") + if dry_run: + print(f"[DRY RUN] Alert email not sent (would go to {alert_email}).") + elif alert_email: + try: + gmail_service = get_gmail_service() + send_alert_email(gmail_service, alert_email, failures, hours, log_dir) + print(f" Alert sent to {alert_email}") + except Exception as e: + print(f" WARNING: failed to send alert email: {e}") + # Clean up temp files for f in all_temp_files: try: os.remove(f) except OSError: pass # Summary print(f"\n{'='*60}") print("DAILY REVIEW COMPLETE") print(f"{'='*60}") print(f"Authors reviewed: {len(reviews)}") print(f"Total commits: {total_commits}") for author_email, data in reviews.items(): - verdict = 'FEEDBACK' if 'OVERALL STATUS: FEEDBACK' in data['review'] else 'APPROVED' - print(f" {data['name']}: {data['num_commits']} commits - {verdict}") + print(f" {data['name']}: {data['num_commits']} commits - {verdict_of(data)}") + + return not failures # ============================================================================= # MAIN # ============================================================================= def save_review(review, ticket_data=None, single_commit=None, standalone_commit=None): """Save review to local file""" if standalone_commit: # Standalone commit review (no ticket) filename = f"code_review_commit_{standalone_commit['short_hash']}.md" elif single_commit: # Single commit within a ticket filename = f"code_review_{ticket_data['ticket_id']}_{ticket_data['coder']}_{single_commit['short_hash']}.md" else: @@ -1437,50 +1538,57 @@ python3 codeReviewAi.py --daily --hours 24 --cc browser-code-reviews-group@ucsc.edu """ ) parser.add_argument('--dry-run', action='store_true', help='Generate reviews but do not post to Redmine / send emails') parser.add_argument('--ticket', type=int, help='Review only this ticket ID') parser.add_argument('--commit', type=str, help='Review a specific commit (can be used with or without --ticket)') parser.add_argument('--daily', action='store_true', help='Daily mode: review recent commits by all authors and email results') parser.add_argument('--hours', type=int, default=24, help='Hours to look back for --daily mode (default: 24)') parser.add_argument('--cc', type=str, default=DEFAULT_CC, help=f'CC address for --daily emails (default: {DEFAULT_CC})') + parser.add_argument('--alert-email', type=str, default=DEFAULT_ALERT_EMAIL, + help=f'Address to alert if a --daily review fails, e.g. an ' + f'expired auth token (default: {DEFAULT_ALERT_EMAIL})') parser.add_argument('--log-dir', type=str, default=os.path.expanduser('~/codeReviewLogs'), help='Directory for daily review logs and output (default: ~/codeReviewLogs)') args = parser.parse_args() # ================================================================= # DAILY MODE (--daily) # ================================================================= if args.daily: - run_daily_mode(args.hours, args.cc, args.dry_run, args.log_dir) - return + ok = run_daily_mode(args.hours, args.cc, args.dry_run, args.log_dir, + alert_email=args.alert_email) + sys.exit(0 if ok else 1) # Load configuration config = load_config() redmine_key = config.get('redmine.apiKey') if not redmine_key: print("ERROR: redmine.apiKey not found in config") sys.exit(1) + # Ticket/commit modes save reviews and debug files under OUTPUT_DIR + os.makedirs(OUTPUT_DIR, exist_ok=True) + # ================================================================= # STANDALONE COMMIT MODE (--commit without --ticket) # ================================================================= if args.commit and not args.ticket: print("=" * 60) print("STANDALONE COMMIT REVIEW") print("=" * 60) # Get commit info from git commit, error = get_commit_from_git(args.commit) if error: print(f"ERROR: {error}") sys.exit(1) print(f"Commit: {commit['short_hash']}")