diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 11deda42..b67ba000 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -22,7 +22,7 @@ "name": "edge-ocp-ci", "source": "./plugins/edge-ocp-ci", "description": "Edge OCP Payload Monitor — monitor OpenShift nightly payloads for edge topology (SNO/TNA/TNF) failures with AI-enriched analysis", - "version": "1.0.2" + "version": "1.2.0" }, { "name": "edge-scrum", @@ -88,7 +88,7 @@ "name": "pr-review", "source": "./plugins/pr-review", "description": "PR lifecycle toolkit — vet findings, triage CodeRabbit reviews, filter noise, and autonomous yolo-agent for CI monitoring and auto-fixes", - "version": "2.0.1" + "version": "2.1.0" }, { "name": "skills-review", diff --git a/payload-monitor/payload_monitor/__main__.py b/payload-monitor/payload_monitor/__main__.py index b840e70c..06811083 100644 --- a/payload-monitor/payload_monitor/__main__.py +++ b/payload-monitor/payload_monitor/__main__.py @@ -9,6 +9,7 @@ from datetime import datetime, timezone from pathlib import Path +import requests import click from .analyzer import analyze @@ -90,15 +91,95 @@ def _collect_jobs_by_type(report: MonitorReport, job_type: JobType) -> list[dict help="Number of payloads to analyze per stream (1-10, default 5)") @click.option("--merge-analysis", "merge_analysis_path", type=click.Path(exists=True), default=None, help="Merge analysis JSON into an existing HTML report (or into --from-json data)") +@click.option("--pr-payload-url", type=str, default=None, + help="Triage a PR payload run (e.g., https://pr-payload-tests.ci.openshift.org/runs/ci/...)") +@click.option("--pr", "pr_ref", type=str, default=None, + help="PR reference for triage (e.g., 'openshift/origin#31276' or full URL)") +@click.option("--format", "output_format", type=click.Choice(["markdown", "json"]), + default="markdown", help="Output format for PR triage (default: markdown)") +@click.option("--discover", is_flag=True, default=False, + help="Discover which payload jobs match the PR's changed files (use with --pr)") def main( versions, output_path, from_json, export_json, open_browser, verbose, skip_prow, skip_sippy, with_timing, - payloads, merge_analysis_path, + payloads, merge_analysis_path, pr_payload_url, pr_ref, output_format, + discover, ): """Edge OCP Payload Monitor — monitor OpenShift nightly payloads for edge topology failures.""" _setup_logging(verbose) logger = logging.getLogger("payload_monitor") + # --- PR payload job discovery mode --- + if discover: + if not pr_ref: + logger.error("--pr is required when using --discover") + raise SystemExit(1) + + from .collectors.payload_job_discovery import ( + discover_payload_jobs, + find_release_repo, + format_discovery_json, + format_discovery_markdown, + ) + from .collectors.pr_payload import fetch_pr_changed_files + + release_repo = find_release_repo() + if not release_repo: + logger.error("Cannot find openshift/release repo. Set RELEASE_REPO_DIR or clone it locally.") + raise SystemExit(1) + + changed_files = fetch_pr_changed_files(pr_ref) + if not changed_files: + logger.error(f"No changed files found for {pr_ref}") + raise SystemExit(1) + + result = discover_payload_jobs(pr_ref, changed_files, release_repo) + + if output_format == "json": + print(format_discovery_json(result)) + else: + print(format_discovery_markdown(result)) + if result.error: + raise SystemExit(1) + return + + # --- PR payload triage mode --- + if pr_payload_url: + if not pr_ref: + logger.error("--pr is required when using --pr-payload-url") + raise SystemExit(1) + + from .collectors.pr_payload import ( + build_triage_result, + classify_failures, + fetch_pr_changed_files, + fetch_pr_diff, + fetch_pr_payload_run, + format_triage_json, + format_triage_markdown, + ) + + try: + jobs = fetch_pr_payload_run(pr_payload_url) + except requests.RequestException as exc: + click.echo(f"Error fetching payload page: {exc}", err=True) + raise SystemExit(1) from exc + if not jobs: + logger.error("No jobs found at the provided URL") + raise SystemExit(1) + + pr_files = fetch_pr_changed_files(pr_ref) + diff_content = fetch_pr_diff(pr_ref) + verdicts = classify_failures(jobs, pr_files, diff_content) + result = build_triage_result(pr_payload_url, pr_ref, jobs) + result.verdicts = verdicts + + if output_format == "json": + print(format_triage_json(result)) + else: + print(format_triage_markdown(result)) + return + # Build config config = Config() if payloads is not None: diff --git a/payload-monitor/payload_monitor/collectors/payload_job_discovery.py b/payload-monitor/payload_monitor/collectors/payload_job_discovery.py new file mode 100644 index 00000000..5afefc5a --- /dev/null +++ b/payload-monitor/payload_monitor/collectors/payload_job_discovery.py @@ -0,0 +1,338 @@ +"""Discover which payload jobs are appropriate for a PR based on changed files. + +Cross-references the PR's changed files against ci-operator configs in the +openshift/release repo to find matching payload jobs via pipeline_run_if_changed +regexes, then constructs the full /payload-job trigger commands. +""" + +from __future__ import annotations + +import json +import logging +import os +import re +import subprocess +from dataclasses import dataclass, field +from pathlib import Path +from typing import Optional + +import yaml + +logger = logging.getLogger(__name__) + + +@dataclass +class PayloadJobSuggestion: + as_name: str + periodic_name: str + trigger_command: str + matched_by: str + matched_files: list[str] = field(default_factory=list) + + +@dataclass +class DiscoveryResult: + pr_ref: str + org: str + repo: str + branch: str + version: str + suggestions: list[PayloadJobSuggestion] = field(default_factory=list) + error: Optional[str] = None + + +def find_release_repo() -> Optional[str]: + """Locate the cloned openshift/release repo.""" + if env_dir := os.environ.get("RELEASE_REPO_DIR"): + if Path(env_dir).is_dir(): + return str(Path(env_dir).resolve()) + logger.warning(f"RELEASE_REPO_DIR set but not found: {env_dir}") + + script_dir = Path(__file__).resolve().parent + candidates = [ + script_dir / "../../../repos/release", + Path.cwd() / "repos/release", + ] + + home = Path.home() + for base in ["Documents/Projects", "Projects", "src", "go/src"]: + candidates.append(home / base / "release") + candidates.append(home / base / "openshift" / "release") + + for c in candidates: + ci_config = c / "ci-operator" / "config" + if ci_config.is_dir(): + return str(c.resolve()) + + return None + + +def _parse_pr_ref(pr_ref: str) -> Optional[tuple[str, str, str]]: + """Parse a PR reference into (org, repo, number).""" + m = re.match( + r'(?:https?://github\.com/)?([^/]+)/([^/#]+)(?:/pull/|#)(\d+)', pr_ref + ) + if not m: + return None + return m.group(1), m.group(2), m.group(3) + + +def _get_pr_branch(org: str, repo: str, number: str) -> str: + """Get the base branch of a PR via gh CLI.""" + try: + result = subprocess.run( + ["gh", "pr", "view", number, "--repo", f"{org}/{repo}", + "--json", "baseRefName", "--jq", ".baseRefName"], + capture_output=True, text=True, timeout=15, + ) + if result.returncode == 0 and result.stdout.strip(): + return result.stdout.strip() + logger.warning("gh pr view failed for %s/%s#%s: %s", org, repo, number, result.stderr.strip()) + except (subprocess.TimeoutExpired, FileNotFoundError): + logger.warning("gh pr view timed out or gh not found for %s/%s#%s", org, repo, number) + return "main" + + +def detect_version(release_repo: str, branch: str) -> Optional[str]: + """Map a branch name to an OCP version. + + release-X.Y → X.Y + main/master → latest nightly config version + """ + m = re.match(r'release-(\d+\.\d+)', branch) + if m: + return m.group(1) + + nightly_dir = Path(release_repo) / "ci-operator/config/openshift/release" + if not nightly_dir.is_dir(): + return None + + pattern = re.compile(r'openshift-release-main__nightly-(\d+\.\d+)\.yaml$') + versions = [] + for f in nightly_dir.iterdir(): + m = pattern.match(f.name) + if m: + versions.append(m.group(1)) + + if not versions: + return None + + def version_key(v: str) -> tuple[int, ...]: + return tuple(int(x) for x in v.split(".")) + + versions.sort(key=version_key) + return versions[-1] + + +def load_repo_ci_config( + release_repo: str, org: str, repo: str, branch: str +) -> list[dict]: + """Load test entries from a repo's ci-operator config in the release repo.""" + config_dir = Path(release_repo) / "ci-operator/config" / org / repo + config_name = f"{org}-{repo}-{branch}.yaml" + config_path = config_dir / config_name + + if not config_path.exists(): + logger.warning(f"CI config not found: {config_path}") + return [] + + with open(config_path) as f: + data = yaml.safe_load(f) + + if not data or "tests" not in data: + return [] + + return data["tests"] + + +def match_changed_files( + test_entries: list[dict], changed_files: list[str] +) -> list[dict]: + """Find test entries whose pipeline_run_if_changed matches any changed file.""" + matched = [] + for entry in test_entries: + regex_str = entry.get("pipeline_run_if_changed") or entry.get("run_if_changed") + if not regex_str: + continue + + try: + pattern = re.compile(regex_str) + except re.error as e: + logger.debug(f"Bad regex in {entry.get('as', '?')}: {e}") + continue + + hits = [f for f in changed_files if pattern.search(f)] + if hits: + matched.append({**entry, "_matched_files": hits, "_matched_by": regex_str}) + + return matched + + +def load_nightly_job_names(release_repo: str, version: str) -> set[str]: + """Load the set of test `as` names from the nightly config for a version.""" + config_path = ( + Path(release_repo) + / "ci-operator/config/openshift/release" + / f"openshift-release-main__nightly-{version}.yaml" + ) + if not config_path.exists(): + logger.warning(f"Nightly config not found: {config_path}") + return set() + + with open(config_path) as f: + data = yaml.safe_load(f) + + if not data or "tests" not in data: + return set() + + return {t["as"] for t in data["tests"] if "as" in t} + + +def validate_existing_command( + trigger_command: str, suggestions: list[PayloadJobSuggestion] +) -> dict: + """Check if an existing /payload-job command matches any discovered job.""" + job_name = trigger_command.replace("/payload-job", "").strip() + + for s in suggestions: + if s.periodic_name == job_name or s.as_name in job_name: + return { + "existing_command": trigger_command, + "is_valid": True, + "note": f"Matches discovered job {s.as_name}", + } + + suggestion_names = [s.as_name for s in suggestions] + return { + "existing_command": trigger_command, + "is_valid": False, + "note": f"Does not match any discovered job. Expected one of: {suggestion_names}", + } + + +def discover_payload_jobs( + pr_ref: str, + changed_files: list[str], + release_repo: str, +) -> DiscoveryResult: + """Discover which payload jobs are appropriate for a PR's changed files.""" + parsed = _parse_pr_ref(pr_ref) + if not parsed: + return DiscoveryResult( + pr_ref=pr_ref, org="", repo="", branch="", version="", + error=f"Could not parse PR reference: {pr_ref}", + ) + org, repo, number = parsed + + branch = _get_pr_branch(org, repo, number) + logger.info(f"PR {org}/{repo}#{number} targets branch: {branch}") + + version = detect_version(release_repo, branch) + if not version: + return DiscoveryResult( + pr_ref=pr_ref, org=org, repo=repo, branch=branch, version="", + error="Could not determine OCP version from branch", + ) + logger.info(f"Detected OCP version: {version}") + + test_entries = load_repo_ci_config(release_repo, org, repo, branch) + if not test_entries: + return DiscoveryResult( + pr_ref=pr_ref, org=org, repo=repo, branch=branch, version=version, + error=f"No CI config found for {org}/{repo} branch {branch}", + ) + logger.info(f"Loaded {len(test_entries)} test entries from CI config") + + matched = match_changed_files(test_entries, changed_files) + if not matched: + return DiscoveryResult( + pr_ref=pr_ref, org=org, repo=repo, branch=branch, version=version, + ) + + nightly_names = load_nightly_job_names(release_repo, version) + logger.info(f"Nightly config has {len(nightly_names)} jobs for version {version}") + + suggestions = [] + for entry in matched: + as_name = entry.get("as", "") + if not as_name: + continue + + if as_name not in nightly_names: + logger.debug(f"Job {as_name} not in nightly config — skipping") + continue + + periodic_name = ( + f"periodic-ci-openshift-release-main-nightly-{version}-{as_name}" + ) + suggestions.append(PayloadJobSuggestion( + as_name=as_name, + periodic_name=periodic_name, + trigger_command=f"/payload-job {periodic_name}", + matched_by=entry["_matched_by"], + matched_files=entry["_matched_files"], + )) + + logger.info(f"Discovered {len(suggestions)} payload job suggestion(s)") + return DiscoveryResult( + pr_ref=pr_ref, + org=org, + repo=repo, + branch=branch, + version=version, + suggestions=suggestions, + ) + + +def format_discovery_json(result: DiscoveryResult) -> str: + """Format discovery result as JSON.""" + output = { + "pr_ref": result.pr_ref, + "org": result.org, + "repo": result.repo, + "branch": result.branch, + "version": result.version, + "suggestions": [ + { + "as_name": s.as_name, + "periodic_name": s.periodic_name, + "trigger_command": s.trigger_command, + "matched_by": s.matched_by, + "matched_files": s.matched_files, + } + for s in result.suggestions + ], + } + if result.error: + output["error"] = result.error + return json.dumps(output, indent=2) + + +def format_discovery_markdown(result: DiscoveryResult) -> str: + """Format discovery result as readable markdown.""" + lines = [] + lines.append("## Payload Job Discovery") + lines.append("") + lines.append(f"**PR:** {result.pr_ref}") + lines.append(f"**Branch:** {result.branch} (OCP {result.version})") + lines.append("") + + if result.error: + lines.append(f"**Error:** {result.error}") + return "\n".join(lines) + + if not result.suggestions: + lines.append("No matching payload jobs found for the changed files.") + return "\n".join(lines) + + lines.append(f"Found **{len(result.suggestions)}** matching payload job(s):") + lines.append("") + + for s in result.suggestions: + lines.append(f"### {s.as_name}") + lines.append(f"**Trigger:** `{s.trigger_command}`") + lines.append(f"**Matched by:** `{s.matched_by}`") + lines.append(f"**Files:** {', '.join(s.matched_files[:5])}") + lines.append("") + + return "\n".join(lines) diff --git a/payload-monitor/payload_monitor/collectors/pr_payload.py b/payload-monitor/payload_monitor/collectors/pr_payload.py new file mode 100644 index 00000000..ed039454 --- /dev/null +++ b/payload-monitor/payload_monitor/collectors/pr_payload.py @@ -0,0 +1,445 @@ +"""Fetch and triage PR payload test runs from pr-payload-tests.ci.openshift.org.""" + +from __future__ import annotations + +import json +import logging +import re +import subprocess +import xml.etree.ElementTree as ET +from concurrent.futures import ThreadPoolExecutor, as_completed +from typing import Optional + +import requests + +from ..models import ( + FailingTest, + FailureVerdict, + JobResult, + PRPayloadJob, + PRTriageResult, +) +from .http import create_session +from .payload_job_discovery import _parse_pr_ref as _parse_pr_ref_3 +from .prow import _fetch_gcs_file, _prow_url_to_gcs_path + +logger = logging.getLogger(__name__) + +_session = create_session() + +# Matches Prow job links embedded in the pr-payload-tests page +_PROW_HREF_RE = re.compile( + r'href="(https://prow\.ci\.openshift\.org/view/gs/[^"]+)"' +) +# Extracts job name and build ID from a Prow URL +_PROW_JOB_RE = re.compile(r'/logs/([^/]+)/(\d+)') + + +def _parse_prow_urls(html: str) -> list[str]: + return list(dict.fromkeys(_PROW_HREF_RE.findall(html))) # deduplicated, order preserved + + +def _job_name_from_url(prow_url: str) -> str: + m = _PROW_JOB_RE.search(prow_url) + return m.group(1) if m else prow_url + + +def _fetch_job_result(prow_url: str) -> JobResult: + gcs_base = _prow_url_to_gcs_path(prow_url) + if not gcs_base: + return JobResult.UNKNOWN + content = _fetch_gcs_file(f"{gcs_base}/finished.json") + if not content: + return JobResult.UNKNOWN + try: + data = json.loads(content) + result = data.get("result", "").upper() + if result == "SUCCESS" or data.get("passed") is True: + return JobResult.SUCCESS + if result in ("FAILURE", "FAILED", "ERROR"): + return JobResult.FAILURE + if result in ("PENDING", "RUNNING"): + return JobResult.PENDING + return JobResult.UNKNOWN + except json.JSONDecodeError: + return JobResult.UNKNOWN + + +# --- Deep junit: fetch individual test results from step artifacts --- + +def _list_gcs_dir(gcs_path: str) -> list[str]: + """List files in a GCS directory.""" + try: + result = subprocess.run( + ["gsutil", "ls", gcs_path], + capture_output=True, text=True, timeout=30, + ) + if result.returncode == 0: + return [line.strip() for line in result.stdout.strip().split("\n") if line.strip()] + return [] + except (subprocess.TimeoutExpired, FileNotFoundError): + return [] + + +def _find_e2e_step_name(gcs_base: str) -> Optional[str]: + """Discover the e2e test step directory name from the artifacts.""" + artifacts_path = f"{gcs_base}/artifacts/" + dirs = _list_gcs_dir(artifacts_path) + for d in dirs: + # The top-level step directory (e.g. e2e-metal-ovn-two-node-fencing-recovery/) + if d.endswith("/") and "gather" not in d and "release" not in d and "build" not in d: + step_name = d.rstrip("/").rsplit("/", 1)[-1] + return step_name + return None + + +def _fetch_deep_junit_failures(prow_url: str) -> list[FailingTest]: + """Fetch individual test-level failures from the deep junit artifacts. + + Looks for junit_e2e__*.xml inside the e2e test step's artifacts/junit/ directory. + These contain the actual Go test names (g.It descriptions), not just step-level results. + """ + gcs_base = _prow_url_to_gcs_path(prow_url) + if not gcs_base: + return [] + + step_name = _find_e2e_step_name(gcs_base) + if not step_name: + return [] + + junit_dir = f"{gcs_base}/artifacts/{step_name}/baremetalds-e2e-test/artifacts/junit/" + files = _list_gcs_dir(junit_dir) + + junit_files = [f for f in files if f.endswith(".xml") and "junit_e2e" in f] + if not junit_files: + # Fall back to any junit XML + junit_files = [f for f in files if f.endswith(".xml")] + + tests: list[FailingTest] = [] + for jf in junit_files: + content = _fetch_gcs_file(jf) + if not content: + continue + try: + root = ET.fromstring(content) + for tc in root.iter("testcase"): + failure = tc.find("failure") + if failure is not None: + name = tc.get("name", "") + error_msg = failure.get("message", "") or (failure.text or "") + error_msg = error_msg.replace(" ", "\n").replace(""", '"') + duration = float(tc.get("time", "0") or "0") + tests.append(FailingTest( + name=name, error_message=error_msg[:500], duration_seconds=duration, + )) + except ET.ParseError as e: + logger.debug(f"Failed to parse {jf}: {e}") + + return tests + + +def _fetch_single_job(prow_url: str) -> PRPayloadJob: + name = _job_name_from_url(prow_url) + result = _fetch_job_result(prow_url) + job = PRPayloadJob(name=name, prow_url=prow_url, result=result) + + if result == JobResult.FAILURE: + # Fetch deep junit for individual test names + deep_tests = _fetch_deep_junit_failures(prow_url) + if deep_tests: + job.failing_tests = deep_tests + job.error_summary = "; ".join(t.name[:80] for t in deep_tests[:3]) + else: + # Fall back to step-level junit + from .prow import _fetch_junit_failures + tests, summary = _fetch_junit_failures(prow_url) + job.failing_tests = tests + job.error_summary = summary + + return job + + +def fetch_pr_payload_run(url: str, max_workers: int = 4) -> list[PRPayloadJob]: + """Fetch a pr-payload-tests run URL and return all jobs with results.""" + logger.info(f"Fetching PR payload run: {url}") + try: + resp = _session.get(url) + resp.raise_for_status() + except requests.RequestException as exc: + logger.warning("Payload page fetch failed: %s", exc) + return [] + + prow_urls = _parse_prow_urls(resp.text) + if not prow_urls: + logger.warning("No Prow job links found on the page") + return [] + + logger.info(f"Found {len(prow_urls)} job(s), fetching results...") + jobs: list[PRPayloadJob] = [] + with ThreadPoolExecutor(max_workers=max_workers) as pool: + futures = {pool.submit(_fetch_single_job, u): u for u in prow_urls} + for future in as_completed(futures): + try: + jobs.append(future.result()) + except Exception as e: + logger.error(f"Failed to fetch job {futures[future]}: {e}") + + jobs.sort(key=lambda j: j.name) + return jobs + + +def build_triage_result( + payload_url: str, + pr_ref: str, + jobs: list[PRPayloadJob], +) -> PRTriageResult: + passed = sum(1 for j in jobs if j.result == JobResult.SUCCESS) + failed = sum(1 for j in jobs if j.result == JobResult.FAILURE) + return PRTriageResult( + payload_url=payload_url, + pr_ref=pr_ref, + total_jobs=len(jobs), + passed=passed, + failed=failed, + jobs=jobs, + ) + + +# --- PR diff fetching --- + +def _parse_pr_ref(pr_ref: str) -> Optional[tuple[str, str]]: + """Parse a PR reference into (repo, number). Returns None if unparsable.""" + parsed = _parse_pr_ref_3(pr_ref) + if not parsed: + return None + org, repo, number = parsed + return f"{org}/{repo}", number + + +def fetch_pr_changed_files(pr_ref: str) -> list[str]: + """Fetch changed file paths from a GitHub PR.""" + parsed = _parse_pr_ref(pr_ref) + if not parsed: + logger.warning(f"Could not parse PR reference: {pr_ref}") + return [] + repo, number = parsed + try: + result = subprocess.run( + ["gh", "api", f"repos/{repo}/pulls/{number}/files", + "--paginate", "--jq", ".[].filename"], + capture_output=True, text=True, timeout=30, + ) + if result.returncode != 0: + logger.error(f"gh api failed: {result.stderr.strip()}") + return [] + return [f for f in result.stdout.strip().split("\n") if f] + except (subprocess.TimeoutExpired, FileNotFoundError) as e: + logger.error(f"Failed to fetch PR files: {e}") + return [] + + +def fetch_pr_diff(pr_ref: str) -> str: + """Fetch the full diff content of a PR.""" + parsed = _parse_pr_ref(pr_ref) + if not parsed: + return "" + repo, number = parsed + try: + result = subprocess.run( + ["gh", "api", f"repos/{repo}/pulls/{number}", + "-H", "Accept: application/vnd.github.v3.diff"], + capture_output=True, text=True, timeout=30, + ) + if result.returncode != 0: + logger.error(f"gh api diff failed: {result.stderr.strip()}") + return "" + return result.stdout + except (subprocess.TimeoutExpired, FileNotFoundError) as e: + logger.error(f"Failed to fetch PR diff: {e}") + return "" + + +# --- Classifier --- + +def _extract_test_description(test_name: str) -> str: + """Extract the g.It description from a full junit test name. + + Junit names look like: + [sig-etcd][...] Two Node with Fencing etcd disruption should start normally as standalone voter... + The meaningful part is after the last ] bracket. + """ + # Strip leading [sig-...][...] tags + stripped = re.sub(r'\[[^\]]*\]', '', test_name).strip() + return stripped + + +def classify_failure( + job: PRPayloadJob, + diff_content: str, + pr_files: list[str], +) -> FailureVerdict: + """Classify whether a job failure is PR-caused or flaky. + + Uses diff-aware matching: checks if the failing test's description + appears in the PR diff, indicating the PR modified or added that test. + """ + diff_lower = diff_content.lower() + + for t in job.failing_tests: + # Skip generic infrastructure failures + if "cluster precondition" in t.name.lower(): + continue + + desc = _extract_test_description(t.name) + if not desc: + continue + + # Check if the test description (or a significant substring) appears in the diff + # Match on a meaningful fragment — at least 20 chars to avoid false positives + fragments = [desc] + if len(desc) > 40: + # Also try the last part (the "should ..." clause) + should_match = re.search(r'(should .+)', desc, re.IGNORECASE) + if should_match: + fragments.append(should_match.group(1)) + + for frag in fragments: + if len(frag) >= 20 and frag.lower() in diff_lower: + matched = [f for f in pr_files if any( + part in f.lower() for part in desc.lower().split()[:3] + )] + return FailureVerdict( + verdict="pr-caused", + reason=f"Failing test found in PR diff: \"{desc}\"", + matched_files=matched if matched else pr_files, + ) + + # Fallback: check if any failing test name references functions/identifiers in the diff + # Extract added/removed Go identifiers from the diff + diff_identifiers = set(re.findall(r'[+-]func (\w+)', diff_content)) + if diff_identifiers: + for t in job.failing_tests: + for ident in diff_identifiers: + if ident.lower() in t.name.lower() or ident.lower() in t.error_message.lower(): + return FailureVerdict( + verdict="pr-caused", + reason=f"Failing test references PR function: {ident}", + matched_files=pr_files, + ) + + return FailureVerdict( + verdict="flaky", + reason="Failing test(s) not found in PR diff — likely unrelated to changes", + ) + + +def classify_failures( + jobs: list[PRPayloadJob], + pr_files: list[str], + diff_content: str, +) -> dict[str, FailureVerdict]: + """Classify all failing jobs. Returns job_name -> verdict mapping.""" + verdicts: dict[str, FailureVerdict] = {} + for job in jobs: + if job.result != JobResult.FAILURE: + continue + verdicts[job.name] = classify_failure(job, diff_content, pr_files) + return verdicts + + +# --- Markdown output --- + +def format_triage_json(result: PRTriageResult) -> str: + """Format a triage result as structured JSON for machine consumption.""" + jobs_out = [] + for job in result.jobs: + verdict = result.verdicts.get(job.name) + job_data = { + "name": job.name, + "result": job.result.name, + "prow_url": job.prow_url, + "verdict": verdict.verdict if verdict else None, + "reason": verdict.reason if verdict else None, + "matched_files": verdict.matched_files if verdict else [], + "failing_tests": [ + {"name": t.name, "error": t.error_message.split("\n")[0][:200]} + for t in job.failing_tests + ], + } + jobs_out.append(job_data) + + unknown_count = sum(1 for j in jobs_out if j["result"] == "UNKNOWN") + complete = unknown_count == 0 + + pr_caused = [j for j in jobs_out if j["verdict"] == "pr-caused"] + if not complete: + recommendation = "Payload jobs still running. Re-check later." + elif not result.failed: + recommendation = "All jobs passed. No triage needed." + elif not pr_caused: + recommendation = "All failures are unrelated to this PR. Safe to re-trigger with /payload-job." + elif len(pr_caused) == len([j for j in jobs_out if j["result"] == "FAILURE"]): + recommendation = "All failures are in tests this PR modifies. Investigate before re-triggering /payload-job." + else: + recommendation = "Some failures are PR-caused. Investigate those before re-triggering /payload-job." + + output = { + "payload_url": result.payload_url, + "pr_ref": result.pr_ref, + "total_jobs": result.total_jobs, + "passed": result.passed, + "failed": result.failed, + "complete": complete, + "jobs": jobs_out, + "recommendation": recommendation, + } + return json.dumps(output, indent=2) + + +def format_triage_markdown(result: PRTriageResult) -> str: + """Format a triage result as a readable markdown summary.""" + lines: list[str] = [] + + status = "PASS" if result.failed == 0 else "FAIL" + lines.append(f"## PR Payload Triage: {status}") + lines.append("") + lines.append(f"**PR:** {result.pr_ref}") + lines.append(f"**Payload run:** {result.payload_url}") + lines.append(f"**Jobs:** {result.passed}/{result.total_jobs} passed, {result.failed} failed") + lines.append("") + + if result.failed == 0: + lines.append("All jobs passed. No triage needed.") + return "\n".join(lines) + + lines.append("### Failure Classification") + lines.append("") + + for job in result.jobs: + if job.result != JobResult.FAILURE: + continue + verdict = result.verdicts.get(job.name) + if not verdict: + continue + + icon = "🔴" if verdict.verdict == "pr-caused" else "🟡" + label = verdict.verdict.upper().replace("-", " ") + lines.append(f"#### {icon} {job.name}") + lines.append(f"**Verdict:** {label}") + lines.append(f"**Reason:** {verdict.reason}") + if verdict.matched_files: + lines.append(f"**Matched files:** {', '.join(verdict.matched_files)}") + lines.append(f"**Prow:** {job.prow_url}") + + if job.failing_tests: + lines.append("") + lines.append("| Test | Error |") + lines.append("|------|-------|") + for t in job.failing_tests: + name = t.name[:120] + error = t.error_message.split("\n")[0][:100] + lines.append(f"| {name} | {error} |") + + lines.append("") + + return "\n".join(lines) diff --git a/payload-monitor/payload_monitor/models.py b/payload-monitor/payload_monitor/models.py index fb4e75f2..9fbb3eb3 100644 --- a/payload-monitor/payload_monitor/models.py +++ b/payload-monitor/payload_monitor/models.py @@ -269,3 +269,32 @@ class TimingReport: @property def successful_runs(self) -> list[TimingRun]: return [r for r in self.runs.values() if r.is_success] + + +# PR payload triage models + +@dataclass +class PRPayloadJob: + name: str + prow_url: str + result: JobResult + failing_tests: list[FailingTest] = field(default_factory=list) + error_summary: str = "" + + +@dataclass +class FailureVerdict: + verdict: str # "pr-caused" | "flaky" | "unknown" + reason: str + matched_files: list[str] = field(default_factory=list) + + +@dataclass +class PRTriageResult: + payload_url: str + pr_ref: str # e.g. "openshift/origin#31276" + total_jobs: int + passed: int + failed: int + jobs: list[PRPayloadJob] = field(default_factory=list) + verdicts: dict[str, FailureVerdict] = field(default_factory=dict) # job_name -> verdict diff --git a/payload-monitor/tests/test_collectors_payload_job_discovery.py b/payload-monitor/tests/test_collectors_payload_job_discovery.py new file mode 100644 index 00000000..c4a45218 --- /dev/null +++ b/payload-monitor/tests/test_collectors_payload_job_discovery.py @@ -0,0 +1,417 @@ +"""Tests for payload_monitor.collectors.payload_job_discovery.""" + +import json +from unittest.mock import patch + +import pytest +import yaml + +from payload_monitor.collectors import payload_job_discovery +from payload_monitor.collectors.payload_job_discovery import ( + DiscoveryResult, + PayloadJobSuggestion, + _parse_pr_ref, + detect_version, + discover_payload_jobs, + format_discovery_json, + format_discovery_markdown, + load_nightly_job_names, + load_repo_ci_config, + match_changed_files, + validate_existing_command, +) + + +class TestParsePrRef: + def test_github_url(self): + result = _parse_pr_ref("https://github.com/openshift/origin/pull/31276") + assert result == ("openshift", "origin", "31276") + + def test_short_ref(self): + result = _parse_pr_ref("openshift/origin#31276") + assert result == ("openshift", "origin", "31276") + + def test_url_with_hash(self): + result = _parse_pr_ref("openshift/installer#10546") + assert result == ("openshift", "installer", "10546") + + def test_invalid_string(self): + assert _parse_pr_ref("not-a-ref") is None + + def test_empty_string(self): + assert _parse_pr_ref("") is None + + def test_url_missing_number(self): + assert _parse_pr_ref("https://github.com/openshift/origin/pull/") is None + + +class TestDetectVersion: + def test_release_branch(self, tmp_path): + assert detect_version(str(tmp_path), "release-4.19") == "4.19" + + def test_release_branch_older(self, tmp_path): + assert detect_version(str(tmp_path), "release-4.17") == "4.17" + + def test_main_branch_latest_version(self, tmp_path): + nightly_dir = tmp_path / "ci-operator/config/openshift/release" + nightly_dir.mkdir(parents=True) + (nightly_dir / "openshift-release-main__nightly-4.18.yaml").write_text( + yaml.dump({"tests": [{"as": "e2e-test"}]}) + ) + (nightly_dir / "openshift-release-main__nightly-4.19.yaml").write_text( + yaml.dump({"tests": [{"as": "e2e-test"}]}) + ) + (nightly_dir / "openshift-release-main__nightly-4.17.yaml").write_text( + yaml.dump({"tests": [{"as": "e2e-test"}]}) + ) + result = detect_version(str(tmp_path), "main") + assert result == "4.19" + + def test_main_no_nightly_dir(self, tmp_path): + assert detect_version(str(tmp_path), "main") is None + + def test_main_empty_nightly_dir(self, tmp_path): + nightly_dir = tmp_path / "ci-operator/config/openshift/release" + nightly_dir.mkdir(parents=True) + assert detect_version(str(tmp_path), "main") is None + + +class TestLoadRepoCiConfig: + def test_valid_config(self, tmp_path): + config_dir = tmp_path / "ci-operator/config/openshift/origin" + config_dir.mkdir(parents=True) + config = { + "tests": [ + {"as": "e2e-test", "pipeline_run_if_changed": "pkg/"}, + {"as": "unit", "run_if_changed": "test/"}, + ] + } + (config_dir / "openshift-origin-main.yaml").write_text(yaml.dump(config)) + + result = load_repo_ci_config(str(tmp_path), "openshift", "origin", "main") + assert len(result) == 2 + assert result[0]["as"] == "e2e-test" + + def test_missing_file(self, tmp_path): + result = load_repo_ci_config(str(tmp_path), "openshift", "origin", "main") + assert result == [] + + def test_no_tests_key(self, tmp_path): + config_dir = tmp_path / "ci-operator/config/openshift/origin" + config_dir.mkdir(parents=True) + (config_dir / "openshift-origin-main.yaml").write_text(yaml.dump({"zz_generated_metadata": {}})) + + result = load_repo_ci_config(str(tmp_path), "openshift", "origin", "main") + assert result == [] + + def test_empty_yaml(self, tmp_path): + config_dir = tmp_path / "ci-operator/config/openshift/origin" + config_dir.mkdir(parents=True) + (config_dir / "openshift-origin-main.yaml").write_text("") + + result = load_repo_ci_config(str(tmp_path), "openshift", "origin", "main") + assert result == [] + + +class TestMatchChangedFiles: + def test_matching_files(self): + entries = [ + {"as": "e2e-fencing", "pipeline_run_if_changed": r"test/extended/two_node/"}, + ] + changed = ["test/extended/two_node/fencing_test.go", "README.md"] + result = match_changed_files(entries, changed) + assert len(result) == 1 + assert result[0]["as"] == "e2e-fencing" + assert result[0]["_matched_files"] == ["test/extended/two_node/fencing_test.go"] + + def test_no_match(self): + entries = [ + {"as": "e2e-fencing", "pipeline_run_if_changed": r"test/extended/two_node/"}, + ] + changed = ["pkg/util/helper.go"] + assert match_changed_files(entries, changed) == [] + + def test_no_regex_entries(self): + entries = [{"as": "unit", "steps": {}}] + assert match_changed_files(entries, ["file.go"]) == [] + + def test_bad_regex(self): + entries = [ + {"as": "bad", "pipeline_run_if_changed": "[invalid(regex"}, + ] + result = match_changed_files(entries, ["anything"]) + assert result == [] + + def test_run_if_changed_fallback(self): + entries = [ + {"as": "e2e", "run_if_changed": r"vendor/"}, + ] + changed = ["vendor/k8s/client.go"] + result = match_changed_files(entries, changed) + assert len(result) == 1 + + def test_multiple_matches(self): + entries = [ + {"as": "e2e-a", "pipeline_run_if_changed": r"pkg/"}, + {"as": "e2e-b", "pipeline_run_if_changed": r"test/"}, + ] + changed = ["pkg/foo.go", "test/bar.go"] + result = match_changed_files(entries, changed) + assert len(result) == 2 + + +class TestLoadNightlyJobNames: + def test_valid_config(self, tmp_path): + nightly_dir = tmp_path / "ci-operator/config/openshift/release" + nightly_dir.mkdir(parents=True) + config = { + "tests": [ + {"as": "e2e-metal-two-node-fencing"}, + {"as": "e2e-metal-single-node"}, + ] + } + (nightly_dir / "openshift-release-main__nightly-4.19.yaml").write_text( + yaml.dump(config) + ) + result = load_nightly_job_names(str(tmp_path), "4.19") + assert result == {"e2e-metal-two-node-fencing", "e2e-metal-single-node"} + + def test_missing_file(self, tmp_path): + assert load_nightly_job_names(str(tmp_path), "4.19") == set() + + def test_no_tests_key(self, tmp_path): + nightly_dir = tmp_path / "ci-operator/config/openshift/release" + nightly_dir.mkdir(parents=True) + (nightly_dir / "openshift-release-main__nightly-4.19.yaml").write_text( + yaml.dump({"metadata": {}}) + ) + assert load_nightly_job_names(str(tmp_path), "4.19") == set() + + +class TestValidateExistingCommand: + def test_matching_periodic_name(self): + suggestions = [ + PayloadJobSuggestion( + as_name="e2e-metal-two-node-fencing", + periodic_name="periodic-ci-openshift-release-main-nightly-4.19-e2e-metal-two-node-fencing", + trigger_command="/payload-job periodic-ci-openshift-release-main-nightly-4.19-e2e-metal-two-node-fencing", + matched_by="regex", + ), + ] + result = validate_existing_command( + "/payload-job periodic-ci-openshift-release-main-nightly-4.19-e2e-metal-two-node-fencing", + suggestions, + ) + assert result["is_valid"] is True + + def test_matching_as_name_substring(self): + suggestions = [ + PayloadJobSuggestion( + as_name="e2e-metal-two-node-fencing", + periodic_name="periodic-ci-openshift-release-main-nightly-4.19-e2e-metal-two-node-fencing", + trigger_command="/payload-job ...", + matched_by="regex", + ), + ] + result = validate_existing_command( + "/payload-job some-prefix-e2e-metal-two-node-fencing", + suggestions, + ) + assert result["is_valid"] is True + + def test_no_match(self): + suggestions = [ + PayloadJobSuggestion( + as_name="e2e-metal-two-node-fencing", + periodic_name="periodic-ci-openshift-release-main-nightly-4.19-e2e-metal-two-node-fencing", + trigger_command="/payload-job ...", + matched_by="regex", + ), + ] + result = validate_existing_command( + "/payload-job some-completely-different-job", + suggestions, + ) + assert result["is_valid"] is False + assert "e2e-metal-two-node-fencing" in result["note"] + + def test_empty_suggestions(self): + result = validate_existing_command("/payload-job foo", []) + assert result["is_valid"] is False + + +class TestDiscoverPayloadJobs: + @patch.object(payload_job_discovery, "_get_pr_branch", return_value="main") + def test_full_discovery(self, mock_branch, tmp_path): + # Set up CI config for openshift/origin + ci_dir = tmp_path / "ci-operator/config/openshift/origin" + ci_dir.mkdir(parents=True) + ci_config = { + "tests": [ + { + "as": "e2e-metal-two-node-fencing", + "pipeline_run_if_changed": r"test/extended/two_node/", + }, + { + "as": "e2e-unrelated", + "pipeline_run_if_changed": r"vendor/", + }, + ] + } + (ci_dir / "openshift-origin-main.yaml").write_text(yaml.dump(ci_config)) + + # Set up nightly config + nightly_dir = tmp_path / "ci-operator/config/openshift/release" + nightly_dir.mkdir(parents=True) + nightly_config = { + "tests": [ + {"as": "e2e-metal-two-node-fencing"}, + {"as": "e2e-metal-single-node"}, + ] + } + (nightly_dir / "openshift-release-main__nightly-4.19.yaml").write_text( + yaml.dump(nightly_config) + ) + + result = discover_payload_jobs( + "openshift/origin#31276", + ["test/extended/two_node/fencing_test.go"], + str(tmp_path), + ) + + assert result.org == "openshift" + assert result.repo == "origin" + assert result.error is None + assert len(result.suggestions) == 1 + assert result.suggestions[0].as_name == "e2e-metal-two-node-fencing" + assert "periodic-ci-openshift-release-main-nightly-4.19" in result.suggestions[0].periodic_name + + def test_invalid_pr_ref(self, tmp_path): + result = discover_payload_jobs("invalid", ["file.go"], str(tmp_path)) + assert result.error is not None + assert "Could not parse" in result.error + + @patch.object(payload_job_discovery, "_get_pr_branch", return_value="main") + def test_no_matching_files(self, mock_branch, tmp_path): + ci_dir = tmp_path / "ci-operator/config/openshift/origin" + ci_dir.mkdir(parents=True) + ci_config = { + "tests": [ + {"as": "e2e", "pipeline_run_if_changed": r"vendor/"}, + ] + } + (ci_dir / "openshift-origin-main.yaml").write_text(yaml.dump(ci_config)) + + nightly_dir = tmp_path / "ci-operator/config/openshift/release" + nightly_dir.mkdir(parents=True) + (nightly_dir / "openshift-release-main__nightly-4.19.yaml").write_text( + yaml.dump({"tests": [{"as": "e2e"}]}) + ) + + result = discover_payload_jobs( + "openshift/origin#1", + ["README.md"], + str(tmp_path), + ) + assert result.error is None + assert result.suggestions == [] + + @patch.object(payload_job_discovery, "_get_pr_branch", return_value="main") + def test_job_not_in_nightly(self, mock_branch, tmp_path): + ci_dir = tmp_path / "ci-operator/config/openshift/origin" + ci_dir.mkdir(parents=True) + ci_config = { + "tests": [ + {"as": "e2e-custom", "pipeline_run_if_changed": r"pkg/"}, + ] + } + (ci_dir / "openshift-origin-main.yaml").write_text(yaml.dump(ci_config)) + + nightly_dir = tmp_path / "ci-operator/config/openshift/release" + nightly_dir.mkdir(parents=True) + (nightly_dir / "openshift-release-main__nightly-4.19.yaml").write_text( + yaml.dump({"tests": [{"as": "e2e-other"}]}) + ) + + result = discover_payload_jobs( + "openshift/origin#1", + ["pkg/foo.go"], + str(tmp_path), + ) + assert result.suggestions == [] + + +class TestFormatDiscoveryJson: + def test_with_suggestions(self): + result = DiscoveryResult( + pr_ref="openshift/origin#1", + org="openshift", + repo="origin", + branch="main", + version="4.19", + suggestions=[ + PayloadJobSuggestion( + as_name="e2e-fencing", + periodic_name="periodic-ci-openshift-release-main-nightly-4.19-e2e-fencing", + trigger_command="/payload-job periodic-ci-...", + matched_by="regex", + matched_files=["test/foo.go"], + ), + ], + ) + output = json.loads(format_discovery_json(result)) + assert output["version"] == "4.19" + assert len(output["suggestions"]) == 1 + assert "error" not in output + + def test_with_error(self): + result = DiscoveryResult( + pr_ref="bad", org="", repo="", branch="", version="", + error="Could not parse", + ) + output = json.loads(format_discovery_json(result)) + assert output["error"] == "Could not parse" + assert output["suggestions"] == [] + + +class TestFormatDiscoveryMarkdown: + def test_with_suggestions(self): + result = DiscoveryResult( + pr_ref="openshift/origin#1", + org="openshift", + repo="origin", + branch="main", + version="4.19", + suggestions=[ + PayloadJobSuggestion( + as_name="e2e-fencing", + periodic_name="periodic-...", + trigger_command="/payload-job periodic-...", + matched_by="regex", + matched_files=["test/foo.go"], + ), + ], + ) + md = format_discovery_markdown(result) + assert "Payload Job Discovery" in md + assert "e2e-fencing" in md + assert "1" in md # "Found 1 matching" + + def test_no_suggestions(self): + result = DiscoveryResult( + pr_ref="openshift/origin#1", + org="openshift", + repo="origin", + branch="main", + version="4.19", + ) + md = format_discovery_markdown(result) + assert "No matching" in md + + def test_with_error(self): + result = DiscoveryResult( + pr_ref="bad", org="", repo="", branch="", version="", + error="Something broke", + ) + md = format_discovery_markdown(result) + assert "Something broke" in md diff --git a/payload-monitor/tests/test_collectors_pr_payload.py b/payload-monitor/tests/test_collectors_pr_payload.py new file mode 100644 index 00000000..3569524e --- /dev/null +++ b/payload-monitor/tests/test_collectors_pr_payload.py @@ -0,0 +1,307 @@ +"""Tests for payload_monitor.collectors.pr_payload.""" + +import json +from unittest.mock import patch + +import pytest + +from payload_monitor.collectors import pr_payload +from payload_monitor.collectors.pr_payload import ( + PRTriageResult, + _extract_test_description, + _job_name_from_url, + _parse_prow_urls, + build_triage_result, + classify_failure, + classify_failures, + format_triage_json, +) +from payload_monitor.models import ( + FailingTest, + FailureVerdict, + JobResult, + PRPayloadJob, +) + + +class TestParseProwUrls: + def test_extracts_urls(self): + html = """ + job-a + job-b + """ + result = _parse_prow_urls(html) + assert len(result) == 2 + assert "job-a/111" in result[0] + assert "job-b/222" in result[1] + + def test_deduplicates(self): + html = """ + 1 + 2 + """ + result = _parse_prow_urls(html) + assert len(result) == 1 + + def test_no_links(self): + assert _parse_prow_urls("no links") == [] + + def test_non_prow_links_ignored(self): + html = 'not prow' + assert _parse_prow_urls(html) == [] + + +class TestJobNameFromUrl: + def test_standard_url(self): + url = "https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-test-job/12345" + assert _job_name_from_url(url) == "periodic-ci-test-job" + + def test_non_matching_url(self): + url = "https://example.com/other" + assert _job_name_from_url(url) == url + + +class TestFetchJobResult: + @patch.object(pr_payload, "_fetch_gcs_file") + @patch.object(pr_payload, "_prow_url_to_gcs_path") + def test_success(self, mock_gcs_path, mock_fetch): + mock_gcs_path.return_value = "gs://bucket/logs/job/123" + mock_fetch.return_value = json.dumps({"result": "SUCCESS", "passed": True}) + + result = pr_payload._fetch_job_result("https://prow/view/gs/bucket/logs/job/123") + assert result == JobResult.SUCCESS + + @patch.object(pr_payload, "_fetch_gcs_file") + @patch.object(pr_payload, "_prow_url_to_gcs_path") + def test_failure(self, mock_gcs_path, mock_fetch): + mock_gcs_path.return_value = "gs://bucket/logs/job/123" + mock_fetch.return_value = json.dumps({"result": "FAILURE"}) + + result = pr_payload._fetch_job_result("https://prow/view/gs/bucket/logs/job/123") + assert result == JobResult.FAILURE + + @patch.object(pr_payload, "_fetch_gcs_file") + @patch.object(pr_payload, "_prow_url_to_gcs_path") + def test_pending(self, mock_gcs_path, mock_fetch): + mock_gcs_path.return_value = "gs://bucket/logs/job/123" + mock_fetch.return_value = json.dumps({"result": "PENDING"}) + + result = pr_payload._fetch_job_result("https://prow/view/gs/bucket/logs/job/123") + assert result == JobResult.PENDING + + @patch.object(pr_payload, "_prow_url_to_gcs_path") + def test_no_gcs_path(self, mock_gcs_path): + mock_gcs_path.return_value = None + result = pr_payload._fetch_job_result("https://example.com") + assert result == JobResult.UNKNOWN + + @patch.object(pr_payload, "_fetch_gcs_file") + @patch.object(pr_payload, "_prow_url_to_gcs_path") + def test_invalid_json(self, mock_gcs_path, mock_fetch): + mock_gcs_path.return_value = "gs://bucket/logs/job/123" + mock_fetch.return_value = "not json" + + result = pr_payload._fetch_job_result("https://prow/view/gs/bucket/logs/job/123") + assert result == JobResult.UNKNOWN + + @patch.object(pr_payload, "_fetch_gcs_file") + @patch.object(pr_payload, "_prow_url_to_gcs_path") + def test_no_finished_file(self, mock_gcs_path, mock_fetch): + mock_gcs_path.return_value = "gs://bucket/logs/job/123" + mock_fetch.return_value = None + + result = pr_payload._fetch_job_result("https://prow/view/gs/bucket/logs/job/123") + assert result == JobResult.UNKNOWN + + +class TestExtractTestDescription: + def test_strips_sig_tags(self): + name = "[sig-etcd][Feature:TwoNode] Two Node with Fencing should start normally" + result = _extract_test_description(name) + assert result == "Two Node with Fencing should start normally" + + def test_multiple_tags(self): + name = "[sig-auth][Feature:OAuth][Serial] OAuth server should handle token" + result = _extract_test_description(name) + assert result == "OAuth server should handle token" + + def test_no_tags(self): + name = "TestSomething" + assert _extract_test_description(name) == "TestSomething" + + def test_empty_string(self): + assert _extract_test_description("") == "" + + +class TestClassifyFailure: + def _make_job(self, test_name, error="error"): + return PRPayloadJob( + name="test-job", + prow_url="https://prow/1", + result=JobResult.FAILURE, + failing_tests=[FailingTest(name=test_name, error_message=error)], + ) + + def test_test_in_diff_pr_caused(self): + job = self._make_job( + "[sig-etcd] Two Node with Fencing should recover after etcd kill" + ) + diff = """ ++ g.It("should recover after etcd kill", func() { ++ // test code ++ }) +""" + verdict = classify_failure(job, diff, ["test/two_node_test.go"]) + assert verdict.verdict == "pr-caused" + + def test_test_not_in_diff_flaky(self): + job = self._make_job( + "[sig-etcd] Two Node with Fencing should recover after etcd kill" + ) + diff = """ ++func helperFunction() { ++ return nil ++} +""" + verdict = classify_failure(job, diff, ["pkg/helper.go"]) + assert verdict.verdict == "flaky" + + def test_func_identifier_match(self): + job = self._make_job( + "[sig-etcd] test", error="helperFunction failed" + ) + diff = """+func helperFunction() error { ++ return nil ++} +""" + verdict = classify_failure(job, diff, ["pkg/helper.go"]) + assert verdict.verdict == "pr-caused" + assert "helperFunction" in verdict.reason + + def test_infra_failure_skipped(self): + job = self._make_job( + "[sig-cluster] cluster precondition check failed" + ) + diff = "+something unrelated in the diff that is long enough to match" + verdict = classify_failure(job, diff, ["file.go"]) + assert verdict.verdict == "flaky" + + def test_short_description_not_matched(self): + job = self._make_job("[sig-x] ab") + diff = "+ab is in the diff" + verdict = classify_failure(job, diff, ["f.go"]) + assert verdict.verdict == "flaky" + + def test_no_failing_tests(self): + job = PRPayloadJob( + name="test-job", + prow_url="https://prow/1", + result=JobResult.FAILURE, + ) + verdict = classify_failure(job, "+diff content", ["file.go"]) + assert verdict.verdict == "flaky" + + +class TestClassifyFailures: + def test_only_failures(self): + jobs = [ + PRPayloadJob("pass-job", "url1", JobResult.SUCCESS), + PRPayloadJob("fail-job", "url2", JobResult.FAILURE, + failing_tests=[FailingTest(name="[sig] test x", error_message="err")]), + ] + verdicts = classify_failures(jobs, ["f.go"], "+unrelated diff content") + assert "fail-job" in verdicts + assert "pass-job" not in verdicts + + def test_empty_jobs(self): + assert classify_failures([], [], "") == {} + + +class TestBuildTriageResult: + def test_counts(self): + jobs = [ + PRPayloadJob("j1", "url1", JobResult.SUCCESS), + PRPayloadJob("j2", "url2", JobResult.FAILURE), + PRPayloadJob("j3", "url3", JobResult.SUCCESS), + PRPayloadJob("j4", "url4", JobResult.FAILURE), + ] + result = build_triage_result("https://payload/run/1", "openshift/origin#1", jobs) + assert result.total_jobs == 4 + assert result.passed == 2 + assert result.failed == 2 + assert result.payload_url == "https://payload/run/1" + + def test_all_pass(self): + jobs = [PRPayloadJob("j1", "url1", JobResult.SUCCESS)] + result = build_triage_result("url", "ref", jobs) + assert result.passed == 1 + assert result.failed == 0 + + def test_empty(self): + result = build_triage_result("url", "ref", []) + assert result.total_jobs == 0 + + +class TestFormatTriageJson: + def test_all_passed(self): + result = PRTriageResult( + payload_url="url", pr_ref="ref", total_jobs=2, passed=2, failed=0, + jobs=[ + PRPayloadJob("j1", "url1", JobResult.SUCCESS), + PRPayloadJob("j2", "url2", JobResult.SUCCESS), + ], + ) + output = json.loads(format_triage_json(result)) + assert output["complete"] is True + assert "No triage needed" in output["recommendation"] + + def test_has_failures_flaky(self): + result = PRTriageResult( + payload_url="url", pr_ref="ref", total_jobs=2, passed=1, failed=1, + jobs=[ + PRPayloadJob("j1", "url1", JobResult.SUCCESS), + PRPayloadJob("j2", "url2", JobResult.FAILURE, + failing_tests=[FailingTest(name="test1", error_message="err")]), + ], + verdicts={"j2": FailureVerdict(verdict="flaky", reason="not in diff")}, + ) + output = json.loads(format_triage_json(result)) + assert output["failed"] == 1 + assert "unrelated" in output["recommendation"].lower() + + def test_has_unknown_jobs(self): + result = PRTriageResult( + payload_url="url", pr_ref="ref", total_jobs=2, passed=1, failed=0, + jobs=[ + PRPayloadJob("j1", "url1", JobResult.SUCCESS), + PRPayloadJob("j2", "url2", JobResult.UNKNOWN), + ], + ) + output = json.loads(format_triage_json(result)) + assert output["complete"] is False + assert "running" in output["recommendation"].lower() + + def test_pr_caused_failures(self): + result = PRTriageResult( + payload_url="url", pr_ref="ref", total_jobs=1, passed=0, failed=1, + jobs=[ + PRPayloadJob("j1", "url1", JobResult.FAILURE, + failing_tests=[FailingTest(name="t", error_message="e")]), + ], + verdicts={"j1": FailureVerdict(verdict="pr-caused", reason="in diff")}, + ) + output = json.loads(format_triage_json(result)) + assert "Investigate" in output["recommendation"] + + +class TestParsePrRefWrapper: + def test_valid_ref(self): + result = pr_payload._parse_pr_ref("openshift/origin#31276") + assert result == ("openshift/origin", "31276") + + def test_github_url(self): + result = pr_payload._parse_pr_ref("https://github.com/openshift/installer/pull/10546") + assert result == ("openshift/installer", "10546") + + def test_invalid(self): + assert pr_payload._parse_pr_ref("invalid") is None diff --git a/plugins/edge-ocp-ci/.claude-plugin/plugin.json b/plugins/edge-ocp-ci/.claude-plugin/plugin.json index 54099c66..6fe5bab3 100644 --- a/plugins/edge-ocp-ci/.claude-plugin/plugin.json +++ b/plugins/edge-ocp-ci/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "edge-ocp-ci", "description": "Edge OCP Payload Monitor — monitor OpenShift nightly payloads for edge topology (SNO/TNA/TNF) failures with AI-enriched analysis", - "version": "1.1.0", + "version": "1.2.0", "author": { "name": "vimauro" }, diff --git a/plugins/edge-ocp-ci/skills/pr-payload-triage/SKILL.md b/plugins/edge-ocp-ci/skills/pr-payload-triage/SKILL.md new file mode 100644 index 00000000..565d2ab3 --- /dev/null +++ b/plugins/edge-ocp-ci/skills/pr-payload-triage/SKILL.md @@ -0,0 +1,92 @@ +--- +name: edge-ocp-ci:pr-payload-triage +description: "Triage a PR payload test run — classify failures as PR-caused vs. flaky by matching failing tests against the PR diff" +argument-hint: " " +user-invocable: true +allowed-tools: + - Bash +--- + +# PR Payload Triage Skill + +You are helping a developer triage the results of a PR payload test run triggered by `/payload-job` on a GitHub PR. The goal is to classify each failing job as either **PR-caused** (the failing test was modified by the PR) or **flaky/unrelated** (the failure is in a test the PR did not touch). + +## Arguments + +The user provides two arguments: + +1. **PR payload URL** — the `pr-payload-tests.ci.openshift.org` link from the openshift-ci bot comment on the PR +2. **PR reference** — the GitHub PR in `owner/repo#number` format (e.g., `openshift/origin#31276`) or a full URL + +If the user provides only the payload URL, ask for the PR reference. If they provide only a PR URL and mention payload, check the PR comments for the payload link. + +## Execution + +Run the payload-monitor CLI in PR triage mode: + +```bash +cd /payload-monitor +python3 -m payload_monitor \ + --pr-payload-url "" \ + --pr "" \ + --verbose +``` + +The tool will: +1. Fetch the pr-payload-tests page and extract Prow job URLs +2. Check each job's pass/fail status via GCS `finished.json` +3. For failing jobs, fetch deep junit artifacts to get individual Go test names +4. Fetch the PR diff from GitHub +5. Classify each failure by checking if the failing test description appears in the diff + +## Output + +The tool prints a markdown triage report to stdout. Present it to the user as-is. + +### Interpreting Results + +| Verdict | Icon | Meaning | +|---------|------|---------| +| PR CAUSED | 🔴 | The failing test was modified by the PR — investigate | +| FLAKY | 🟡 | The failure is in a test the PR did not touch — safe to `/retest` | + +### Follow-up Actions + +Based on the results, suggest: + +- **All FLAKY**: "All failures are unrelated to your PR. Safe to re-trigger." Then find the original `/payload-job` trigger comment on the PR and ask the user to confirm before posting it as a PR comment; never auto-post. +- **Some PR CAUSED**: "Job X failed in a test your PR modified. Check the error and Prow link before re-triggering." +- **All PR CAUSED**: "All failures are in tests your PR touches. Investigate before re-triggering." + +> **Note:** To re-trigger, copy the original `/payload-job ` comment from the PR — not just `/payload-job` alone, and never `/retest` (which only re-runs regular CI checks). + +## Error Handling + +If `payload-monitor` exits non-zero, report the error to the user: + +- **Payload page expired or unavailable**: HTTP error fetching the page. Ask the user to verify the URL is still valid (payload runs expire after a few days). +- **No jobs found on the page**: The URL resolved but contained no Prow job links. The run may still be initializing — suggest waiting a few minutes and retrying. +- **`gh` auth failure**: GitHub API access rejected. Ask the user to run `gh auth login` and re-authenticate. +- **`gsutil` auth failure**: GCS artifact lookups fail; individual jobs show as `UNKNOWN`. Ask the user to run `gcloud auth login` and retry. +- **Tool error (non-zero exit)**: Re-run with `--verbose` to see diagnostic output, then report the error message to the user. + +Between phases, guard checks: +1. After fetching the payload page — verify job URLs were found before proceeding. +2. After GCS lookups — note any `UNKNOWN` results (auth or connectivity issue) before classifying. +3. After fetching the PR diff — if `gh` returns no diff, warn the user and skip classification (do not auto-classify as FLAKY). + +## Prerequisites + +- `python3` with `requests`, `click` installed +- `gsutil` for GCS artifact access +- `gh` CLI authenticated for GitHub API access +- The `edge-tooling` repository cloned locally + +## Examples + +```bash +# Triage from a PR payload link +python3 -m payload_monitor \ + --pr-payload-url "https://pr-payload-tests.ci.openshift.org/runs/ci/2a276860-70a0-11f1-9741-fc7a909b3142-0" \ + --pr "openshift/origin#31276" +``` diff --git a/plugins/pr-review/.claude-plugin/plugin.json b/plugins/pr-review/.claude-plugin/plugin.json index 36245db8..e67e4c06 100644 --- a/plugins/pr-review/.claude-plugin/plugin.json +++ b/plugins/pr-review/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "pr-review", "description": "PR lifecycle toolkit — vet findings, triage CodeRabbit reviews, filter noise, and autonomous yolo-agent for CI monitoring and auto-fixes", - "version": "2.0.1", + "version": "2.1.0", "author": { "name": "fonta-rh, vmauro" }, "homepage": "https://github.com/openshift-eng/edge-tooling", "license": "Apache-2.0" diff --git a/plugins/pr-review/scripts/pr-payload.sh b/plugins/pr-review/scripts/pr-payload.sh new file mode 100755 index 00000000..361547f2 --- /dev/null +++ b/plugins/pr-review/scripts/pr-payload.sh @@ -0,0 +1,262 @@ +#!/usr/bin/bash +set -euo pipefail + +# Detect PR payload test runs, discover appropriate payload jobs, and triage +# failures via payload-monitor. +# +# Three-phase flow: +# Phase 1: Discovery — find which payload jobs match the PR's changed files +# Phase 2: Validation — check if existing /payload-job comments match discovery +# Phase 3: Triage — analyze payload test results if a run exists +# +# Exit codes: 0=results found (triage or discovery), 1=no matches, 3=error + +URL_PATTERN='^https://github\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/pull/[0-9]+$' +PAYLOAD_URL_PATTERN='https://pr-payload-tests\.ci\.openshift\.org/runs/ci/[^ )"'"'"']*' + +die() { + echo "Error: $1" >&2 + exit 3 +} + +check_dependencies() { + command -v gh >/dev/null 2>&1 || die "gh CLI is not installed" + command -v jq >/dev/null 2>&1 || die "jq is not installed" + command -v python3 >/dev/null 2>&1 || die "python3 is not installed" + command -v gsutil >/dev/null 2>&1 || die "gsutil is not installed" + gh auth status >/dev/null 2>&1 || die "gh CLI is not authenticated — run 'gh auth login'" +} + +validate_url() { + local url="$1" + if [[ ! "${url}" =~ ${URL_PATTERN} ]]; then + die "Invalid PR URL: ${url}" + fi +} + +parse_url() { + local url="$1" + ORG=$(echo "${url}" | cut -d'/' -f4) + REPO=$(echo "${url}" | cut -d'/' -f5) + PR_NUMBER=$(echo "${url}" | cut -d'/' -f7) +} + +find_payload_monitor_dir() { + if [[ -n "${PAYLOAD_MONITOR_DIR:-}" ]]; then + if [[ -d "${PAYLOAD_MONITOR_DIR}" ]]; then + echo "${PAYLOAD_MONITOR_DIR}" + return + fi + die "PAYLOAD_MONITOR_DIR set but does not exist: ${PAYLOAD_MONITOR_DIR}" + fi + + local script_dir + script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + + local candidates=( + "${script_dir}/../../../payload-monitor" + "${PWD}/repos/edge-tooling/payload-monitor" + ) + + local base_dirs=("${HOME}/Documents/Projects" "${HOME}/Projects" "${HOME}/src") + for base in "${base_dirs[@]}"; do + candidates+=("${base}/edge-tooling/payload-monitor") + done + + for candidate in "${candidates[@]}"; do + if [[ -d "${candidate}" ]]; then + (cd "${candidate}" && pwd) + return + fi + done + + die "Cannot find payload-monitor directory. Set PAYLOAD_MONITOR_DIR or run from edge-tooling repo." +} + +fetch_pr_comments() { + local org="$1" repo="$2" pr_number="$3" + + gh api "repos/${org}/${repo}/issues/${pr_number}/comments" \ + --paginate 2>/dev/null \ + || die "Failed to fetch PR comments for ${org}/${repo}#${pr_number}" +} + +fetch_latest_payload_url() { + local comments_json="$1" + + echo "${comments_json}" | jq -r '.[].body' 2>/dev/null \ + | grep -oE "${PAYLOAD_URL_PATTERN}" | tail -1 +} + +fetch_trigger_command() { + local comments_json="$1" + + echo "${comments_json}" | jq -r '.[].body' 2>/dev/null \ + | grep -oE '^/payload-job .+' | tail -1 +} + +fetch_payload_comment_timestamp() { + local comments_json="$1" + + echo "${comments_json}" | jq -r ' + [.[] | select(.body | test("^/payload-job "))] | last | .created_at // empty + ' 2>/dev/null +} + +fetch_latest_commit_timestamp() { + local org="$1" repo="$2" pr_number="$3" + + gh pr view "${pr_number}" --repo "${org}/${repo}" \ + --json commits --jq '.commits[-1].committedDate' 2>/dev/null +} + +run_discovery() { + local pr_ref="$1" pm_dir="$2" + + local discovery_output + discovery_output=$(cd "${pm_dir}" && python3 -m payload_monitor \ + --discover --pr "${pr_ref}" --format json 2>/dev/null) || return 1 + + echo "${discovery_output}" | jq -c '.' >/dev/null 2>&1 || return 1 + echo "${discovery_output}" +} + +run_triage() { + local payload_url="$1" pr_ref="$2" pm_dir="$3" + + local triage_output + triage_output=$(cd "${pm_dir}" && python3 -m payload_monitor \ + --pr-payload-url "${payload_url}" \ + --pr "${pr_ref}" \ + --format json 2>/dev/null) || return 1 + + echo "${triage_output}" | jq -c '.' >/dev/null 2>&1 || return 1 + echo "${triage_output}" +} + +main() { + [[ $# -lt 1 ]] && die "Usage: $(basename "$0") " + + local pr_url="$1" + + check_dependencies + validate_url "${pr_url}" + parse_url "${pr_url}" + + local pm_dir + pm_dir=$(find_payload_monitor_dir) + + local pr_ref="${ORG}/${REPO}#${PR_NUMBER}" + + # Fetch comments once — reused by discovery, validation, and triage + local comments + comments=$(fetch_pr_comments "${ORG}" "${REPO}" "${PR_NUMBER}") + + # --- Phase 1: Discovery --- + local discovery_json="" + discovery_json=$(run_discovery "${pr_ref}" "${pm_dir}" 2>/dev/null || true) + + # --- Phase 2: Validate existing /payload-job comments --- + local existing_cmd validation_json="" + existing_cmd=$(fetch_trigger_command "${comments}" 2>/dev/null || true) + if [[ -n "${existing_cmd}" && -n "${discovery_json}" ]]; then + local suggestion_names + suggestion_names=$(echo "${discovery_json}" | jq -r '.suggestions[].periodic_name' 2>/dev/null || true) + + local job_name + job_name="${existing_cmd#/payload-job }" + + local is_valid="false" + local note="" + while IFS= read -r name; do + if [[ -n "${name}" && "${job_name}" == *"${name}"* ]]; then + is_valid="true" + note="Matches discovered job ${name}" + break + fi + done <<< "${suggestion_names}" + + if [[ "${is_valid}" == "false" ]]; then + note="Does not match any discovered job for the changed files" + fi + + validation_json=$(jq -n \ + --arg cmd "${existing_cmd}" \ + --argjson valid "${is_valid}" \ + --arg note "${note}" \ + '{existing_command: $cmd, is_valid: $valid, note: $note}') + fi + + # --- Phase 3: Triage (if payload URL exists) --- + local payload_url + payload_url=$(fetch_latest_payload_url "${comments}" 2>/dev/null || true) + + if [[ -n "${payload_url}" ]]; then + # Staleness check — compare payload comment time vs latest commit + local is_stale="false" + local stale_reason="" + local payload_comment_ts latest_commit_ts + payload_comment_ts=$(fetch_payload_comment_timestamp "${comments}" 2>/dev/null || true) + latest_commit_ts=$(fetch_latest_commit_timestamp "${ORG}" "${REPO}" "${PR_NUMBER}" 2>/dev/null || true) + + if [[ -n "${payload_comment_ts}" && -n "${latest_commit_ts}" ]]; then + if [[ "${latest_commit_ts}" > "${payload_comment_ts}" ]]; then + is_stale="true" + stale_reason="New commit pushed after payload was triggered (commit: ${latest_commit_ts}, trigger: ${payload_comment_ts})" + fi + fi + + # Triage mode — we have actual payload run results + local triage_json + if ! triage_json=$(run_triage "${payload_url}" "${pr_ref}" "${pm_dir}"); then + # Triage failed (expired page, no jobs found) — fall through to discovery + payload_url="" + fi + fi + + if [[ -n "${payload_url}" ]]; then + # Build combined output in a single jq call + local disc_arg="null" + if [[ -n "${discovery_json}" ]]; then + disc_arg=$(echo "${discovery_json}" | jq '{suggestions: .suggestions, version: .version}') + fi + local val_arg="null" + if [[ -n "${validation_json}" ]]; then + val_arg="${validation_json}" + fi + + echo "${triage_json}" | jq \ + --arg mode "triage" \ + --argjson stale "${is_stale}" \ + --arg stale_reason "${stale_reason}" \ + --argjson disc "${disc_arg}" \ + --argjson val "${val_arg}" \ + --arg cmd "${existing_cmd}" \ + '. + {mode: $mode, stale: $stale} + | if $stale then . + {stale_reason: $stale_reason} else . end + | if $disc != null then . + {discovery: $disc} else . end + | if $val != null then . + {validation: $val} else . end + | if $cmd != "" then . + {trigger_command: $cmd} else . end' + exit 0 + fi + + # No payload URL — discovery-only mode + if [[ -n "${discovery_json}" ]]; then + local suggestion_count + suggestion_count=$(echo "${discovery_json}" | jq '.suggestions | length' 2>/dev/null || echo "0") + + if [[ "${suggestion_count}" -gt 0 ]]; then + # Discovery found matching jobs + local output + output=$(echo "${discovery_json}" | jq '. + {mode: "discovery"}') + echo "${output}" + exit 0 + fi + fi + + # Nothing found — no payload URL, no matching jobs + echo '{"found": false, "mode": "none", "reason": "No payload URL in comments and no matching payload jobs discovered"}' + exit 1 +} + +main "$@" diff --git a/plugins/pr-review/skills/yolo-agent/SKILL.md b/plugins/pr-review/skills/yolo-agent/SKILL.md index 2e998890..b81ae24c 100644 --- a/plugins/pr-review/skills/yolo-agent/SKILL.md +++ b/plugins/pr-review/skills/yolo-agent/SKILL.md @@ -111,6 +111,20 @@ include human comments. Extract the branch name from the checks JSON (`.pr.branch`) for use in push operations. Increment the cycle counter via `pr-state.sh increment cycle`. Display a compact status summary. +**Payload detection:** Run `pr-payload.sh `. The script runs +discovery (which payload jobs match the PR's changed files), validates +any existing `/payload-job` comments, and triages results if a payload +run exists. Store the JSON output for step 2c. + +Exit codes: +- `0` = results found — either triage results or discovery suggestions +- `1` = no payload URL AND no matching payload jobs — skip Payload Track +- `3` = error — follow **Error Handling** rules + +The JSON output includes a `mode` field: `"triage"`, `"discovery"`, or +`"none"`. Both `triage` and `discovery` modes require presenting results +in step 2c. + The output includes a `resurfaced` flag on each comment and split summary counts (`total_new`, `total_resurfaced`). Resurfaced comments are threads where the root ID was already in the `addressed` list but a new reply @@ -121,7 +135,10 @@ appeared from someone other than the authenticated user. Check in order: 1. **PR closed/merged** → set status `complete`, clean state file, stop -2. **All CI green AND no new comments AND no resurfaced comments** → +2. **All CI green AND no new comments AND no resurfaced comments AND + payload is clean** — where "payload is clean" means: mode is `"none"`, + OR mode is `"triage"` with `complete: true` and zero `"pr-caused"` + verdicts (all failures are `"flaky"`, unrelated to the PR's changes) → post a PR comment to add the label (see format below), set status `complete`, clean state file, report "PR is ready", stop. Use `gh pr comment --repo /` with this body: @@ -132,12 +149,15 @@ Check in order: This message is AI generated by the yolo-agent of [edge-tooling](https://github.com/openshift-eng/edge-tooling). ``` -3. **New comments OR resurfaced comments OR CI failures** → continue to 2c -4. **Only pending CI, no comments** → skip to 2f +3. **New comments OR resurfaced comments OR CI failures OR payload mode + is `"discovery"` OR payload triage has any `"pr-caused"` verdict** → + continue to 2c +4. **Only pending CI OR payload triage in progress (`complete: false`) + with no `"pr-caused"` verdicts yet; no new comments** → skip to 2f #### 2c: Dispatch Parallel Analysis -Launch up to TWO parallel Agent calls: +Launch up to THREE parallel Agent calls: **Comment Track** (if new or resurfaced comments exist): @@ -228,6 +248,80 @@ skill: Classify each failure as **infrastructure** (recommend retrigger) or **code** (propose fix as trivial or non-trivial). +**Payload Track** (if `pr-payload.sh` returned exit 0 in step 2a): + +This track is **completely independent** from the CI Track. Payload jobs +and regular CI checks are separate systems. Report Payload Track results +in their own section — never merge with CI Track. + +**Do NOT check the `analyzed` list for payload URLs. Do NOT skip the +Payload Track. The script runs every cycle and returns fresh data. +Always present the results.** + +Check the `mode` field in the JSON output to determine which sub-flow +to follow: + +**Triage mode** (`mode: "triage"`): + +The JSON contains: +- `complete` — `true` when all jobs have final results, `false` when + jobs are still running +- `jobs[]` — each with `verdict` (`pr-caused` or `flaky`), `reason`, + `prow_url`, `failing_tests` +- `recommendation` — the action to suggest to the user +- `trigger_command` — the exact command for re-triggering +- `stale` — `true` if a new commit was pushed after the payload was + triggered, `false` otherwise +- `stale_reason` — explanation of the staleness (timestamps) +- `discovery.suggestions[]` — the payload jobs the tool discovered as + appropriate for this PR's **current** changed files +- `validation` — whether the existing `/payload-job` command matches + the discovered jobs. If `is_valid: false`, warn the user. + +**Check `stale` first to decide how to present results:** + +When `stale: false` — results are current. Present the triage verdicts +as the Payload Track report. **Do NOT reinterpret, override, or +second-guess the verdicts.** The tool's diff-aware classification is +authoritative. When suggesting a re-trigger, ask the user to confirm +before posting the exact `trigger_command` string as a PR comment. +Never auto-post. NOT just `/payload-job` and NOT `/retest`. + +When `stale: true` — a new commit was pushed after the payload was +triggered, so the results are from old code. Present the old triage +results as reference (prefix with "Stale results from previous +commit:"), then decide based on `validation`: + +- If `validation.is_valid: true` — the existing command still covers + the current changes. Suggest re-triggering the same `trigger_command`. + Ask the user before posting. +- If `validation.is_valid: false` — the new commit changed which files + are in the PR, so the old command no longer matches. Present the + correct commands from `discovery.suggestions[]` and ask which to + trigger. + +In both stale cases, **ask the user before posting** — same interactive +flow as discovery mode. Never auto-post. + +**Discovery mode** (`mode: "discovery"`): + +No payload run exists yet. The JSON contains: +- `suggestions[]` — payload jobs that match the PR's changed files, + each with `as_name`, `trigger_command`, and `matched_files` +- `version` — the OCP version the jobs target + +Present the discovered payload jobs to the user and **ask whether to +trigger one or more of them**. Use a direct question in the output: + +> "These payload jobs match your PR's changes: +> 1. `` — +> 2. `` — +> +> Would you like me to post any of these as a comment on the PR?" + +Do NOT auto-post `/payload-job` in discovery mode — always ask first. +If the user confirms, post the selected command(s) as PR comment(s). + #### 2d: Apply Fixes For each proposed change, run ALL security checks before applying: @@ -354,6 +448,7 @@ direct `jq` state manipulation, manual `git push`, or ad-hoc replacements.** | `pr-checks.sh` | Fetch PR metadata + CI status | `` | 0=all pass, 1=failures, 2=pending only, 3=error | | `pr-comments.sh` | Fetch unresolved review comments (re-surfaces addressed threads with new replies) | ` [addressed-ids] [--include-users]` | 0=has comments, 1=no comments, 3=error | | `pr-push.sh` | Validate fork remote + push | ` [message --expected-files f1,f2]` | 0=pushed, 1=nothing to push, 3=error | +| `pr-payload.sh` | Detect payload URL in PR comments + triage | `` | 0=payload found, 1=no payload, 3=error | **Mandatory usage rules:** @@ -361,6 +456,7 @@ direct `jq` state manipulation, manual `git push`, or ad-hoc replacements.** - CI check gathering → `pr-checks.sh` (never call `gh pr checks` directly) - Comment gathering → `pr-comments.sh` (never call `gh api` for comments directly) - Pushing changes → `pr-push.sh` (never call `git push` directly — the script validates the fork remote, blocks security-sensitive file patterns, and prevents pushing to upstream). When committing, `--expected-files` is mandatory — the script refuses to commit without it +- Payload triage → `pr-payload.sh` (never scan comments for payload URLs or run the Python tool directly) - Replying to PR comments → use the exact endpoint below (do NOT vary the path): @@ -382,5 +478,6 @@ to `/tmp/pr-review-yolo-agent-.json`. - `gh` CLI authenticated with repo access - `jq` installed - CI analysis skills: `ci:prow-job-analyze-test-failure`, `ci:prow-job-analyze-install-failure` +- Payload triage: `pr-payload.sh` (requires `python3`, `gsutil`, `payload-monitor` in edge-tooling) - Comment analysis skills: `pr-review:coderabbit`, `pr-review:vet-review` - Local clone of the target repository