From 4573fa48108a144b767de08589df38b604f059e9 Mon Sep 17 00:00:00 2001 From: Ernest Osiak Date: Mon, 2 Mar 2026 17:57:29 +0100 Subject: [PATCH] OPS-24091 add Jira Cloud ticket context Fetch Jira issue context using env-based Jira Cloud email/token and inject into related_tickets; update AUTO1_PATCHES ticket IDs. --- AUTO1_PATCHES.md | 9 +- pr_agent/tickets/__init__.py | 4 + pr_agent/tickets/jira_cloud.py | 524 ++++++++++++++++++ pr_agent/tools/pr_config.py | 3 +- pr_agent/tools/pr_description.py | 15 +- pr_agent/tools/pr_reviewer.py | 42 +- pr_agent/tools/ticket_pr_compliance_check.py | 41 +- .../test_jira_cloud_ticket_context.py | 280 ++++++++++ .../unittest/test_pr_reviewer_ticket_note.py | 41 ++ 9 files changed, 940 insertions(+), 19 deletions(-) create mode 100644 pr_agent/tickets/__init__.py create mode 100644 pr_agent/tickets/jira_cloud.py create mode 100644 tests/unittest/test_jira_cloud_ticket_context.py create mode 100644 tests/unittest/test_pr_reviewer_ticket_note.py diff --git a/AUTO1_PATCHES.md b/AUTO1_PATCHES.md index f579c39166..2ee10cff71 100644 --- a/AUTO1_PATCHES.md +++ b/AUTO1_PATCHES.md @@ -14,10 +14,11 @@ Keep this list minimal to ease upstream rebases. | Patch ID | Ticket | Type | Files | Why | Upstream status | Removal criteria | | --- | --- | --- | --- | --- | --- | --- | -| improve-score-rubric | OPS-54090 | Enhancement | pr_agent/settings/code_suggestions/pr_code_suggestions_reflect_prompts.toml | Clarify /improve scoring so merge-blocking issues score 9-10 and map to High severity. | Not upstreamed yet. | Remove once upstream clarifies the scoring rubric for merge-blocking issues. | -| review-severity-emoji | OPS-54090 | Enhancement | pr_agent/algo/utils.py | Add a header emoji for findings severity summary in PR review output. | Not upstreamed yet. | Remove once upstream adds a default emoji for findings severity summary. | -| improve-reflect-fallback-score | OPS-54090 | Fix | pr_agent/tools/pr_code_suggestions.py | Prevent inline suggestion spam when self-reflection fails by defaulting all suggestion scores to 0. | Not upstreamed yet. | Remove once upstream handles reflect failure without inflating scores. | -| describe-mermaid-sanitize | OPS-54090 | Fix | pr_agent/tools/pr_description.py | Strip backticks from Mermaid diagrams to avoid GitHub render failures. | Not upstreamed yet. | Remove once upstream sanitizes Mermaid diagrams or fixes prompt output. | +| improve-score-rubric | OPS-24090 | Enhancement | pr_agent/settings/code_suggestions/pr_code_suggestions_reflect_prompts.toml | Clarify /improve scoring so merge-blocking issues score 9-10 and map to High severity. | Not upstreamed yet. | Remove once upstream clarifies the scoring rubric for merge-blocking issues. | +| review-severity-emoji | OPS-24090 | Enhancement | pr_agent/algo/utils.py | Add a header emoji for findings severity summary in PR review output. | Not upstreamed yet. | Remove once upstream adds a default emoji for findings severity summary. | +| improve-reflect-fallback-score | OPS-24090 | Fix | pr_agent/tools/pr_code_suggestions.py | Prevent inline suggestion spam when self-reflection fails by defaulting all suggestion scores to 0. | Not upstreamed yet. | Remove once upstream handles reflect failure without inflating scores. | +| describe-mermaid-sanitize | OPS-24090 | Fix | pr_agent/tools/pr_description.py | Strip backticks from Mermaid diagrams to avoid GitHub render failures. | Not upstreamed yet. | Remove once upstream sanitizes Mermaid diagrams or fixes prompt output. | +| jira-cloud-ticket-context | OPS-24091 | Enhancement | pr_agent/tickets/jira_cloud.py, pr_agent/tools/ticket_pr_compliance_check.py, pr_agent/tools/pr_reviewer.py, pr_agent/tools/pr_description.py, pr_agent/tools/pr_config.py, tests/unittest/test_jira_cloud_ticket_context.py, tests/unittest/test_pr_reviewer_ticket_note.py | Fetch Jira Cloud ticket context using PR title/branch keys, support both basic auth and scoped-token Jira Cloud API access, and show a review note when the Jira ticket cannot be fetched. | Not upstreamed yet. | Remove once upstream supports Jira Cloud ticket context from PR title/branch, scoped-token auth, and comment-level fetch-failure notes. | ## Rebase checklist diff --git a/pr_agent/tickets/__init__.py b/pr_agent/tickets/__init__.py new file mode 100644 index 0000000000..ed0b7ca79f --- /dev/null +++ b/pr_agent/tickets/__init__.py @@ -0,0 +1,4 @@ +"""Ticket system integrations. + +This package contains optional adapters for external ticket systems (e.g., Jira). +""" diff --git a/pr_agent/tickets/jira_cloud.py b/pr_agent/tickets/jira_cloud.py new file mode 100644 index 0000000000..97b8e22a76 --- /dev/null +++ b/pr_agent/tickets/jira_cloud.py @@ -0,0 +1,524 @@ +import json +import os +import re +from dataclasses import dataclass +from typing import Any, Dict, List, Optional +from urllib.parse import urlparse, urlunparse + +import aiohttp +import html2text + + +def _get_logger(): + # Lazy import to avoid circular imports during Dynaconf initialization. + try: + from pr_agent.log import get_logger + + return get_logger() + except Exception: + import logging + + return logging.getLogger(__name__) + + +MAX_TICKET_CHARACTERS = 10000 +DEFAULT_TIMEOUT_SECONDS = 10 + + +_JIRA_KEY_RE = re.compile(r"^[A-Z][A-Z0-9]{1,9}-[1-9]\d{0,6}$") + +# Prefix-only extraction: +# - PR title example: "OPS-1234 new feature", allow minor variants like "[OPS-1234] ..." or "OPS-1234: ...". +_TITLE_PREFIX_RE = re.compile(r"^\s*[\[(]?(?P[A-Z][A-Z0-9]{1,9}-[1-9]\d{0,6})[\])]?([\s:]|$)") +_BRANCH_PREFIX_RE = re.compile(r"^(?P[A-Z][A-Z0-9]{1,9}-[1-9]\d{0,6})(?:[-_/].*)?$") + + +@dataclass(frozen=True) +class JiraCloudAuth: + site_base_url: str + api_base_url: str + api_token: str + auth_mode: str + email: str = "" + cloud_id: str = "" + + +@dataclass(frozen=True) +class JiraTicketFetchResult: + status: str + key: Optional[str] = None + ticket: Optional[Dict[str, Any]] = None + note: str = "" + + +def _normalize_jira_base_url(raw_url: str) -> str: + raw_url = (raw_url or "").strip() + if not raw_url: + return "" + + if not raw_url.startswith(("http://", "https://")): + raw_url = f"https://{raw_url}" + + parsed = urlparse(raw_url) + # Keep only scheme + netloc + path (drop query/fragment) + scheme = parsed.scheme or "https" + netloc = parsed.netloc + path = (parsed.path or "").rstrip("/") + + # Cloud UI sometimes uses the /jira path. REST APIs are served from the root. + if netloc.endswith(".atlassian.net") and path == "/jira": + path = "" + + normalized = urlunparse((scheme, netloc, path, "", "", "")).rstrip("/") + return normalized + + +def _build_jira_api_base_url(cloud_id: str) -> str: + return f"https://api.atlassian.com/ex/jira/{cloud_id}" + + +def _load_auth_from_env() -> Optional[JiraCloudAuth]: + base_url = _normalize_jira_base_url(os.getenv("JIRA_BASE_URL", "")) + email = (os.getenv("JIRA_API_EMAIL", "") or "").strip() + api_token = (os.getenv("JIRA_API_TOKEN", "") or "").strip() + cloud_id = (os.getenv("JIRA_CLOUD_ID", "") or "").strip() + auth_type = (os.getenv("JIRA_AUTH_TYPE", "") or "").strip().lower() + + if not base_url or not api_token: + return None + + # Scoped Atlassian API tokens use api.atlassian.com URLs with Basic auth and require a cloud id. + # Implicit scoped detection: if both JIRA_CLOUD_ID and JIRA_API_EMAIL are set, prefer scoped mode. + if auth_type == "scoped" or (not auth_type and cloud_id and email): + if not email or not cloud_id: + return None + return JiraCloudAuth( + site_base_url=base_url, + api_base_url=_build_jira_api_base_url(cloud_id), + api_token=api_token, + auth_mode="scoped", + email=email, + cloud_id=cloud_id, + ) + + if auth_type in {"bearer", "oauth", "oauth2", "oauth_3lo"}: + return JiraCloudAuth( + site_base_url=base_url, + api_base_url=_build_jira_api_base_url(cloud_id) if cloud_id else "", + api_token=api_token, + auth_mode="bearer", + email=email, + cloud_id=cloud_id, + ) + + if not email: + return None + + return JiraCloudAuth( + site_base_url=base_url, + api_base_url=base_url, + api_token=api_token, + auth_mode="basic", + email=email, + ) + + +def _has_any_jira_env() -> bool: + return any( + ( + (os.getenv("JIRA_BASE_URL", "") or "").strip(), + (os.getenv("JIRA_API_EMAIL", "") or "").strip(), + (os.getenv("JIRA_API_TOKEN", "") or "").strip(), + (os.getenv("JIRA_CLOUD_ID", "") or "").strip(), + (os.getenv("JIRA_AUTH_TYPE", "") or "").strip(), + ) + ) + + +def extract_jira_ticket_key_from_pr_title(pr_title: str) -> Optional[str]: + if not pr_title: + return None + match = _TITLE_PREFIX_RE.match(pr_title.strip()) + if not match: + return None + return match.group("key") + + +def extract_jira_ticket_key_from_branch(branch: str) -> Optional[str]: + if not branch: + return None + branch = branch.strip() + if branch.startswith("refs/heads/"): + branch = branch[len("refs/heads/") :] + match = _BRANCH_PREFIX_RE.match(branch) + if not match: + return None + return match.group("key") + + +def extract_jira_ticket_key(pr_title: str, branch: str) -> Optional[str]: + # precedence: PR title first, then branch + return extract_jira_ticket_key_from_pr_title(pr_title) or extract_jira_ticket_key_from_branch(branch) + + +def _clip_text(text: str, limit: int = MAX_TICKET_CHARACTERS) -> str: + text = text or "" + if len(text) <= limit: + return text + return text[:limit] + "..." + + +def _adf_to_text(adf: Any) -> str: + """Best-effort extraction of human-readable text from Atlassian Document Format (ADF).""" + parts: List[str] = [] + + def walk(node: Any, depth: int = 0): + if depth > 50: + return + if node is None: + return + if isinstance(node, str): + parts.append(node) + return + if isinstance(node, list): + for item in node: + walk(item, depth + 1) + return + if isinstance(node, dict): + node_type = node.get("type") + if node_type == "text" and "text" in node: + parts.append(str(node.get("text") or "")) + return + content = node.get("content") + if content is not None: + walk(content, depth + 1) + # Add lightweight paragraph separation when we see block-ish nodes + if node_type in {"paragraph", "heading", "blockquote", "listItem"}: + parts.append("\n") + return + + walk(adf) + text = "".join(parts) + # Normalize excessive whitespace/newlines + text = re.sub(r"\n{3,}", "\n\n", text) + return text.strip() + + +def _jira_issue_url(base_url: str, key: str) -> str: + return f"{base_url.rstrip('/')}/browse/{key}" + + +def _extract_resource_urls(resources: Any) -> List[str]: + urls: List[str] = [] + if not isinstance(resources, list): + return urls + for resource in resources: + if not isinstance(resource, dict): + continue + url = resource.get("url") + if isinstance(url, str) and url: + urls.append(_normalize_jira_base_url(url)) + return urls + + +def _build_ticket_compliance_note(key: str, reason: str) -> str: + reason_to_message = { + "missing_config": "Jira Cloud configuration is missing", + "not_found": "the Jira ticket was not found", + "auth_failed": "Jira authentication failed", + "fetch_error": "the Jira ticket could not be fetched", + } + message = reason_to_message.get(reason, "the Jira ticket could not be fetched") + return ( + f"Jira ticket `{key}` was detected in the PR title or branch, but {message}, " + "so ticket compliance analysis was skipped." + ) + + +def _truncate_for_log(value: str, limit: int = 300) -> str: + value = (value or "").strip() + if len(value) <= limit: + return value + return value[:limit] + "..." + + +async def _extract_error_message(resp: aiohttp.ClientResponse) -> str: + try: + body = await resp.text() + except Exception: + return "" + + body = (body or "").replace("\n", " ").strip() + if not body: + return "" + + try: + parsed = json.loads(body) + except Exception: + return _truncate_for_log(body) + + error_messages = parsed.get("errorMessages") if isinstance(parsed, dict) else None + if isinstance(error_messages, list) and error_messages: + return _truncate_for_log("; ".join([str(message) for message in error_messages if message])) + + errors = parsed.get("errors") if isinstance(parsed, dict) else None + if isinstance(errors, dict) and errors: + return _truncate_for_log("; ".join([f"{k}: {v}" for k, v in errors.items()])) + + message = parsed.get("message") if isinstance(parsed, dict) else None + if message: + return _truncate_for_log(str(message)) + + return _truncate_for_log(body) + + +async def _discover_cloud_id(session: aiohttp.ClientSession, site_base_url: str) -> Optional[str]: + discovery_url = "https://api.atlassian.com/oauth/token/accessible-resources" + normalized_site_base_url = _normalize_jira_base_url(site_base_url) + _get_logger().info(f"Discovering Jira cloud id for {normalized_site_base_url}") + + async with session.get(discovery_url) as resp: + if resp.status in (401, 403): + error_message = await _extract_error_message(resp) + if error_message: + raise PermissionError(f"Jira cloud id discovery failed (HTTP {resp.status}): {error_message}") + raise PermissionError(f"Jira cloud id discovery failed (HTTP {resp.status})") + resp.raise_for_status() + resources = await resp.json() + + for resource in resources if isinstance(resources, list) else []: + if not isinstance(resource, dict): + continue + resource_url = _normalize_jira_base_url(str(resource.get("url") or "")) + if resource_url == normalized_site_base_url: + cloud_id = str(resource.get("id") or "").strip() + if cloud_id: + _get_logger().info(f"Discovered Jira cloud id {cloud_id} for {normalized_site_base_url}") + return cloud_id + + resource_urls = _extract_resource_urls(resources) + _get_logger().warning( + f"Could not match Jira site {normalized_site_base_url} in accessible resources: {resource_urls}" + ) + return None + + +async def _resolve_auth(session: aiohttp.ClientSession, auth: JiraCloudAuth) -> Optional[JiraCloudAuth]: + if auth.auth_mode == "scoped": + # api_base_url and cloud_id are validated by _load_auth_from_env for scoped tokens. + return auth + + if auth.auth_mode != "bearer": + return auth + + if auth.cloud_id and auth.api_base_url: + _get_logger().info(f"Using configured Jira cloud id {auth.cloud_id} for {auth.site_base_url}") + return auth + + cloud_id = await _discover_cloud_id(session, auth.site_base_url) + if not cloud_id: + return None + + return JiraCloudAuth( + site_base_url=auth.site_base_url, + api_base_url=_build_jira_api_base_url(cloud_id), + api_token=auth.api_token, + auth_mode=auth.auth_mode, + email=auth.email, + cloud_id=cloud_id, + ) + + +async def _fetch_issue_json(session: aiohttp.ClientSession, auth: JiraCloudAuth, key: str) -> Optional[Dict[str, Any]]: + url = f"{auth.api_base_url.rstrip('/')}/rest/api/3/issue/{key}" + params = { + "fields": "summary,description,labels,subtasks", + "expand": "renderedFields", + } + + _get_logger().info( + f"Requesting Jira ticket {key} using {auth.auth_mode} auth from {auth.api_base_url}" + ) + + async with session.get(url, params=params) as resp: + if resp.status == 404: + error_message = await _extract_error_message(resp) + if error_message: + _get_logger().warning( + f"Jira issue lookup for {key} at {auth.api_base_url} returned 404: {error_message}" + ) + else: + _get_logger().warning(f"Jira issue lookup for {key} at {auth.api_base_url} returned 404") + return None + if resp.status in (401, 403): + # Include Jira's capped error message in logs for auth diagnostics. + error_message = await _extract_error_message(resp) + if error_message: + raise PermissionError(f"Jira auth failed (HTTP {resp.status}): {error_message}") + raise PermissionError(f"Jira auth failed (HTTP {resp.status})") + if resp.status >= 400: + error_message = await _extract_error_message(resp) + if error_message: + _get_logger().warning( + f"Jira issue lookup for {key} at {auth.api_base_url} returned HTTP {resp.status}: {error_message}" + ) + else: + _get_logger().warning(f"Jira issue lookup for {key} at {auth.api_base_url} returned HTTP {resp.status}") + resp.raise_for_status() + return await resp.json() + + +def _render_description_to_text(issue: Dict[str, Any]) -> str: + rendered_fields = issue.get("renderedFields") or {} + rendered_html = rendered_fields.get("description") + if isinstance(rendered_html, str) and rendered_html.strip(): + try: + # html2text produces markdown-ish plain text, which is a good prompt format. + return html2text.html2text(rendered_html).strip() + except Exception: + # Fall back to raw HTML if conversion fails. + return rendered_html + + fields = issue.get("fields") or {} + desc = fields.get("description") + if isinstance(desc, str): + return desc + if isinstance(desc, (dict, list)): + text = _adf_to_text(desc) + if text: + return text + return json.dumps(desc, ensure_ascii=True) + return "" + + +def _extract_labels(issue: Dict[str, Any]) -> str: + fields = issue.get("fields") or {} + labels = fields.get("labels") or [] + if not isinstance(labels, list): + return "" + return ", ".join(str(label) for label in labels if label is not None) + + +def _extract_subtasks_as_sub_issues(issue: Dict[str, Any], base_url: str, limit: int = 5) -> List[Dict[str, str]]: + fields = issue.get("fields") or {} + subtasks = fields.get("subtasks") or [] + if not isinstance(subtasks, list) or not subtasks: + return [] + + sub_issues: List[Dict[str, str]] = [] + for subtask in subtasks[:limit]: + if not isinstance(subtask, dict): + continue + key = subtask.get("key") + if not isinstance(key, str) or not _JIRA_KEY_RE.match(key): + continue + summary = "" + sub_fields = subtask.get("fields") + if isinstance(sub_fields, dict): + summary_val = sub_fields.get("summary") + if isinstance(summary_val, str): + summary = summary_val + sub_issues.append( + { + "ticket_url": _jira_issue_url(base_url, key), + "title": summary or key, + "body": "", + } + ) + + return sub_issues + + +async def fetch_jira_ticket_context(pr_title: str, branch: str) -> JiraTicketFetchResult: + key = extract_jira_ticket_key(pr_title, branch) + if not key: + _get_logger().info("No Jira ticket key found in PR title or branch") + return JiraTicketFetchResult(status="no_key") + + _get_logger().info(f"Detected Jira ticket key {key} from PR metadata") + + auth = _load_auth_from_env() + if not auth: + if not _has_any_jira_env(): + _get_logger().info(f"Detected Jira ticket key {key}, but Jira integration is not configured") + return JiraTicketFetchResult(status="not_configured", key=key) + _get_logger().warning(f"Detected Jira ticket key {key}, but Jira Cloud env configuration is missing") + return JiraTicketFetchResult( + status="missing_config", + key=key, + note=_build_ticket_compliance_note(key, "missing_config"), + ) + + timeout = aiohttp.ClientTimeout(total=DEFAULT_TIMEOUT_SECONDS) + session_kwargs: Dict[str, Any] = {"timeout": timeout, "headers": {"Accept": "application/json"}} + if auth.auth_mode in {"basic", "scoped"}: + session_kwargs["auth"] = aiohttp.BasicAuth(login=auth.email, password=auth.api_token) + else: + session_kwargs["headers"].update({ + "Authorization": f"Bearer {auth.api_token}", + }) + _get_logger().info(f"Using Jira auth mode {auth.auth_mode} for {auth.site_base_url}") + + try: + async with aiohttp.ClientSession(**session_kwargs) as session: + resolved_auth = await _resolve_auth(session, auth) + if not resolved_auth: + return JiraTicketFetchResult( + status="fetch_error", + key=key, + note=_build_ticket_compliance_note(key, "fetch_error"), + ) + + issue = await _fetch_issue_json(session, resolved_auth, key) + if not issue: + _get_logger().warning(f"Jira ticket {key} was not found") + return JiraTicketFetchResult( + status="not_found", + key=key, + note=_build_ticket_compliance_note(key, "not_found"), + ) + + fields = issue.get("fields") or {} + summary = fields.get("summary") if isinstance(fields.get("summary"), str) else "" + body = _clip_text(_render_description_to_text(issue)) + labels = _extract_labels(issue) + sub_issues = _extract_subtasks_as_sub_issues(issue, resolved_auth.site_base_url) + + ticket = { + "ticket_id": key, + "ticket_url": _jira_issue_url(resolved_auth.site_base_url, key), + "title": summary or key, + "body": body, + "labels": labels, + } + if sub_issues: + ticket["sub_issues"] = sub_issues + _get_logger().info(f"Fetched Jira ticket {key} for PR ticket context") + return JiraTicketFetchResult(status="fetched", key=key, ticket=ticket) + except PermissionError as e: + # Do not include raw exception details that could accidentally expose auth state. + _get_logger().warning(f"Jira ticket fetch skipped: {e}") + return JiraTicketFetchResult( + status="auth_failed", + key=key, + note=_build_ticket_compliance_note(key, "auth_failed"), + ) + except aiohttp.ClientResponseError as e: + _get_logger().warning(f"Jira ticket fetch failed (HTTP {e.status}) for {key}") + return JiraTicketFetchResult( + status="fetch_error", + key=key, + note=_build_ticket_compliance_note(key, "fetch_error"), + ) + except Exception as e: + _get_logger().warning(f"Jira ticket fetch failed for {key}: {type(e).__name__}") + return JiraTicketFetchResult( + status="fetch_error", + key=key, + note=_build_ticket_compliance_note(key, "fetch_error"), + ) + + +async def try_fetch_jira_ticket(pr_title: str, branch: str) -> Optional[Dict[str, Any]]: + result = await fetch_jira_ticket_context(pr_title, branch) + return result.ticket diff --git a/pr_agent/tools/pr_config.py b/pr_agent/tools/pr_config.py index 24ecaab97b..e16f2aef3d 100644 --- a/pr_agent/tools/pr_config.py +++ b/pr_agent/tools/pr_config.py @@ -57,7 +57,8 @@ def _prepare_pr_configs(self) -> str: skip_keys = ['ai_disclaimer', 'ai_disclaimer_title', 'ANALYTICS_FOLDER', 'secret_provider', "skip_keys", "app_id", "redirect", 'trial_prefix_message', 'no_eligible_message', 'identity_provider', 'ALLOWED_REPOS', 'APP_NAME', 'PERSONAL_ACCESS_TOKEN', 'shared_secret', 'key', 'AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY', 'user_token', - 'private_key', 'private_key_id', 'client_id', 'client_secret', 'token', 'bearer_token', 'jira_api_token','webhook_secret'] + 'private_key', 'private_key_id', 'client_id', 'client_secret', 'token', 'bearer_token', 'jira_api_email', + 'jira_api_token', 'jira_base_url', 'jira_cloud_id', 'webhook_secret'] partial_skip_keys = ['key', 'secret', 'token', 'private'] extra_skip_keys = get_settings().config.get('config.skip_keys', []) if extra_skip_keys: diff --git a/pr_agent/tools/pr_description.py b/pr_agent/tools/pr_description.py index c629da98fa..15797d6934 100644 --- a/pr_agent/tools/pr_description.py +++ b/pr_agent/tools/pr_description.py @@ -26,8 +26,7 @@ from pr_agent.log import get_logger from pr_agent.servers.help import HelpMessage from pr_agent.tools.ticket_pr_compliance_check import ( - extract_and_cache_pr_tickets, extract_ticket_links_from_pr_description, - extract_tickets) + extract_and_cache_pr_tickets, extract_ticket_links_from_pr_description) class PRDescription: @@ -71,7 +70,8 @@ def __init__(self, pr_url: str, args: list = None, "enable_custom_labels": get_settings().config.enable_custom_labels, "custom_labels_class": "", # will be filled if necessary in 'set_custom_labels' function "enable_semantic_files_types": get_settings().pr_description.enable_semantic_files_types, - "related_tickets": "", + "related_tickets": [], + "ticket_compliance_note": "", "include_file_summary_changes": len(self.git_provider.get_diff_files()) <= self.COLLAPSIBLE_FILE_LIST_THRESHOLD, "duplicate_prompt_examples": get_settings().config.get("duplicate_prompt_examples", False), "enable_pr_diagram": enable_pr_diagram, @@ -129,6 +129,15 @@ async def run(self): if not self.git_provider.is_supported( "publish_file_comments") or not get_settings().pr_description.inline_file_summary: pr_body += "\n\n" + changes_walkthrough + "___\n\n" + + ticket_compliance_note = (self.vars.get("ticket_compliance_note") or "").strip() + if ticket_compliance_note: + if self.git_provider.is_supported("gfm_markdown"): + quoted_note = ticket_compliance_note.replace("\n", "\n> ") + pr_body = f"> **🎫 Ticket compliance status**\n>\n> {quoted_note}\n\n{pr_body}" + else: + pr_body = f"### 🎫 Ticket compliance status\n\n{ticket_compliance_note}\n\n{pr_body}" + get_logger().debug("PR output", artifact={"title": pr_title, "body": pr_body}) # Add help text if gfm_markdown is supported diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index c4917f3597..30548171b0 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -23,8 +23,34 @@ get_main_pr_language) from pr_agent.log import get_logger from pr_agent.servers.help import HelpMessage -from pr_agent.tools.ticket_pr_compliance_check import ( - extract_and_cache_pr_tickets, extract_tickets) +from pr_agent.tools.ticket_pr_compliance_check import extract_and_cache_pr_tickets + + +def inject_ticket_compliance_note(markdown_text: str, ticket_note: str, gfm_supported: bool) -> str: + markdown_text = markdown_text or "" + ticket_note = (ticket_note or "").strip() + if not ticket_note: + return markdown_text + + if gfm_supported: + note_row = ( + "\n\n" + "**🎫 Ticket compliance status**\n\n" + f"{ticket_note}\n\n" + "\n" + ) + if "\n" in markdown_text: + return markdown_text.replace("
\n", f"
\n{note_row}", 1) + return f"{markdown_text}\n\n
\n{note_row}
\n".strip() + + note_section = f"### 🎫 Ticket compliance status\n\n{ticket_note}\n\n" + if not markdown_text: + return note_section.strip() + + parts = markdown_text.split("\n\n", 1) + if len(parts) == 2: + return f"{parts[0]}\n\n{note_section}{parts[1]}" + return f"{markdown_text}\n\n{note_section}".strip() class PRReviewer: @@ -97,6 +123,7 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False, "enable_custom_labels": get_settings().config.enable_custom_labels, "is_ai_metadata": get_settings().get("config.enable_ai_metadata", False), "related_tickets": get_settings().get('related_tickets', []), + "ticket_compliance_note": "", 'duplicate_prompt_examples': get_settings().config.get('duplicate_prompt_examples', False), "date": datetime.datetime.now().strftime('%Y-%m-%d'), } @@ -256,9 +283,14 @@ def _prepare_pr_review(self) -> str: incremental_review_markdown_text = f"Starting from commit {last_commit_url}" markdown_text = convert_to_markdown_v2(data, self.git_provider.is_supported("gfm_markdown"), - incremental_review_markdown_text, - git_provider=self.git_provider, - files=self.git_provider.get_diff_files()) + incremental_review_markdown_text, + git_provider=self.git_provider, + files=self.git_provider.get_diff_files()) + markdown_text = inject_ticket_compliance_note( + markdown_text, + self.vars.get("ticket_compliance_note", ""), + self.git_provider.is_supported("gfm_markdown"), + ) # Add help text if gfm_markdown is supported if self.git_provider.is_supported("gfm_markdown") and get_settings().pr_reviewer.enable_help_text: diff --git a/pr_agent/tools/ticket_pr_compliance_check.py b/pr_agent/tools/ticket_pr_compliance_check.py index 523e21f921..8215abb162 100644 --- a/pr_agent/tools/ticket_pr_compliance_check.py +++ b/pr_agent/tools/ticket_pr_compliance_check.py @@ -5,6 +5,7 @@ from pr_agent.git_providers import GithubProvider from pr_agent.git_providers import AzureDevopsProvider from pr_agent.log import get_logger +from pr_agent.tickets.jira_cloud import fetch_jira_ticket_context # Compile the regex pattern once, outside the function GITHUB_TICKET_PATTERN = re.compile( @@ -64,13 +65,32 @@ def extract_ticket_links_from_pr_description(pr_description, repo_path, base_url return list(github_tickets) -async def extract_tickets(git_provider): +async def extract_tickets(git_provider) -> tuple[list | None, str]: MAX_TICKET_CHARACTERS = 10000 + jira_ticket_compliance_note = "" try: + tickets_content = [] + + # Jira Cloud ticket extraction (prefix-only) from PR title, then branch. + # Supports basic, scoped-token, and explicit bearer/OAuth env configurations. + try: + pr_title = "" + if hasattr(git_provider, 'get_title'): + pr_title = git_provider.get_title() or "" + elif hasattr(git_provider, 'pr') and hasattr(git_provider.pr, 'title'): + pr_title = git_provider.pr.title or "" + pr_branch = git_provider.get_pr_branch() if hasattr(git_provider, 'get_pr_branch') else "" + jira_result = await fetch_jira_ticket_context(pr_title, pr_branch) + if jira_result.ticket: + tickets_content.append(jira_result.ticket) + elif jira_result.note: + jira_ticket_compliance_note = jira_result.note + except Exception as e: + get_logger().warning(f"Failed to extract Jira ticket: {type(e).__name__}") + if isinstance(git_provider, GithubProvider): user_description = git_provider.get_user_description() tickets = extract_ticket_links_from_pr_description(user_description, git_provider.repo, git_provider.base_url_html) - tickets_content = [] if tickets: @@ -130,11 +150,10 @@ async def extract_tickets(git_provider): 'sub_issues': sub_issues_content # Store sub-issues content }) - return tickets_content + return tickets_content or None, jira_ticket_compliance_note elif isinstance(git_provider, AzureDevopsProvider): tickets_info = git_provider.get_linked_work_items() - tickets_content = [] for ticket in tickets_info: try: ticket_body_str = ticket.get("body", "") @@ -156,21 +175,31 @@ async def extract_tickets(git_provider): f"Error processing Azure DevOps ticket: {e}", artifact={"traceback": traceback.format_exc()}, ) - return tickets_content + return tickets_content, jira_ticket_compliance_note + + if tickets_content: + return tickets_content, jira_ticket_compliance_note except Exception as e: get_logger().error(f"Error extracting tickets error= {e}", artifact={"traceback": traceback.format_exc()}) + return None, jira_ticket_compliance_note + async def extract_and_cache_pr_tickets(git_provider, vars): if not get_settings().get('pr_reviewer.require_ticket_analysis_review', False): return + vars['ticket_compliance_note'] = "" + related_tickets = get_settings().get('related_tickets', []) if not related_tickets: - tickets_content = await extract_tickets(git_provider) + tickets_content, jira_ticket_compliance_note = await extract_tickets(git_provider) + + if jira_ticket_compliance_note: + vars['ticket_compliance_note'] = jira_ticket_compliance_note if tickets_content: # Store sub-issues along with main issues diff --git a/tests/unittest/test_jira_cloud_ticket_context.py b/tests/unittest/test_jira_cloud_ticket_context.py new file mode 100644 index 0000000000..104929f76c --- /dev/null +++ b/tests/unittest/test_jira_cloud_ticket_context.py @@ -0,0 +1,280 @@ +import asyncio + +from pr_agent.tickets import jira_cloud + + +class TestJiraCloudTicketKeyExtraction: + def test_extract_from_pr_title_prefix(self): + assert jira_cloud.extract_jira_ticket_key("OPS-1234 new feature", "") == "OPS-1234" + + def test_reject_zero_issue_number(self): + assert jira_cloud.extract_jira_ticket_key("OPS-0 invalid", "OPS-0-invalid") is None + + def test_extract_from_branch_prefix(self): + assert jira_cloud.extract_jira_ticket_key("", "OPS-1234-new-feature") == "OPS-1234" + + def test_precedence_title_over_branch(self): + assert jira_cloud.extract_jira_ticket_key("OPS-1111 something", "OPS-2222-new") == "OPS-1111" + + def test_branch_refs_heads_prefix(self): + assert jira_cloud.extract_jira_ticket_key("", "refs/heads/OPS-1234-new") == "OPS-1234" + + def test_normalize_atlassian_jira_path(self): + assert ( + jira_cloud._normalize_jira_base_url("https://wkdauto.atlassian.net/jira") == "https://wkdauto.atlassian.net" + ) + + +class TestJiraCloudFetch: + def test_extract_error_message_from_jira_error_payload(self): + class FakeResponse: + async def text(self): + return '{"errorMessages": ["Issue does not exist or you do not have permission to see it."]}' + + message = asyncio.run(jira_cloud._extract_error_message(FakeResponse())) + assert message == "Issue does not exist or you do not have permission to see it." + + def test_render_description_to_text_from_adf(self): + issue = { + "renderedFields": {}, + "fields": { + "description": { + "type": "doc", + "content": [ + { + "type": "paragraph", + "content": [ + {"type": "text", "text": "Hello"}, + {"type": "text", "text": " Jira"}, + ], + } + ], + } + }, + } + + assert jira_cloud._render_description_to_text(issue) == "Hello Jira" + + def test_try_fetch_jira_ticket_maps_fields(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net/jira") + monkeypatch.setenv("JIRA_API_EMAIL", "user@example.com") + monkeypatch.setenv("JIRA_API_TOKEN", "token") + + async def fake_fetch_issue_json(session, auth, key): + assert auth.site_base_url == "https://wkdauto.atlassian.net" + assert auth.api_base_url == "https://wkdauto.atlassian.net" + assert auth.auth_mode == "basic" + assert key == "OPS-1234" + return { + "fields": { + "summary": "Implement Jira support", + "labels": ["backend", "review"], + "subtasks": [ + {"key": "OPS-2", "fields": {"summary": "Subtask 1"}}, + {"key": "OPS-3", "fields": {"summary": "Subtask 2"}}, + ], + }, + "renderedFields": {"description": "

Hello world

"}, + } + + monkeypatch.setattr(jira_cloud, "_fetch_issue_json", fake_fetch_issue_json) + + ticket = asyncio.run(jira_cloud.try_fetch_jira_ticket("OPS-1234 new feature", "OPS-9999-ignored")) + assert ticket is not None + assert ticket["ticket_id"] == "OPS-1234" + assert ticket["ticket_url"] == "https://wkdauto.atlassian.net/browse/OPS-1234" + assert ticket["title"] == "Implement Jira support" + assert "Hello" in ticket["body"] + assert ticket["labels"] == "backend, review" + + assert "sub_issues" in ticket + assert ticket["sub_issues"][0]["ticket_url"] == "https://wkdauto.atlassian.net/browse/OPS-2" + assert ticket["sub_issues"][0]["title"] == "Subtask 1" + assert ticket["sub_issues"][0]["body"] == "" + + def test_fetch_jira_ticket_context_uses_scoped_token_with_cloud_id(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net") + monkeypatch.setenv("JIRA_API_EMAIL", "user@example.com") + monkeypatch.setenv("JIRA_API_TOKEN", "token") + monkeypatch.setenv("JIRA_AUTH_TYPE", "scoped") + monkeypatch.setenv("JIRA_CLOUD_ID", "2672de8d-bce7-4cb5-9e65-128f96b0bd9a") + + async def fake_fetch_issue_json(session, auth, key): + assert auth.auth_mode == "scoped" + assert auth.site_base_url == "https://wkdauto.atlassian.net" + assert auth.api_base_url == "https://api.atlassian.com/ex/jira/2672de8d-bce7-4cb5-9e65-128f96b0bd9a" + assert key == "OPS-1234" + return { + "fields": { + "summary": "Implement Jira support", + "labels": [], + "subtasks": [], + }, + "renderedFields": {"description": "

Hello world

"}, + } + + monkeypatch.setattr(jira_cloud, "_fetch_issue_json", fake_fetch_issue_json) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-9999-ignored")) + assert result.status == "fetched" + assert result.ticket is not None + assert result.ticket["ticket_url"] == "https://wkdauto.atlassian.net/browse/OPS-1234" + + def test_fetch_jira_ticket_context_uses_scoped_token_with_implicit_detection(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net") + monkeypatch.setenv("JIRA_API_EMAIL", "user@example.com") + monkeypatch.setenv("JIRA_API_TOKEN", "token") + monkeypatch.delenv("JIRA_AUTH_TYPE", raising=False) + monkeypatch.setenv("JIRA_CLOUD_ID", "2672de8d-bce7-4cb5-9e65-128f96b0bd9a") + + async def fake_fetch_issue_json(session, auth, key): + assert auth.auth_mode == "scoped" + assert auth.api_base_url == "https://api.atlassian.com/ex/jira/2672de8d-bce7-4cb5-9e65-128f96b0bd9a" + return { + "fields": { + "summary": "Implement Jira support", + "labels": [], + "subtasks": [], + }, + "renderedFields": {"description": "

Hello world

"}, + } + + monkeypatch.setattr(jira_cloud, "_fetch_issue_json", fake_fetch_issue_json) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-9999-ignored")) + assert result.status == "fetched" + assert result.ticket is not None + + def test_fetch_jira_ticket_context_discovers_cloud_id_for_bearer_token(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net") + monkeypatch.setenv("JIRA_API_TOKEN", "token") + monkeypatch.setenv("JIRA_AUTH_TYPE", "oauth") + monkeypatch.delenv("JIRA_CLOUD_ID", raising=False) + monkeypatch.delenv("JIRA_API_EMAIL", raising=False) + + async def fake_discover_cloud_id(session, site_base_url): + assert site_base_url == "https://wkdauto.atlassian.net" + return "2672de8d-bce7-4cb5-9e65-128f96b0bd9a" + + async def fake_fetch_issue_json(session, auth, key): + assert auth.auth_mode == "bearer" + assert auth.api_base_url == "https://api.atlassian.com/ex/jira/2672de8d-bce7-4cb5-9e65-128f96b0bd9a" + return { + "fields": { + "summary": "Implement Jira support", + "labels": [], + "subtasks": [], + }, + "renderedFields": {"description": "

Hello world

"}, + } + + monkeypatch.setattr(jira_cloud, "_discover_cloud_id", fake_discover_cloud_id) + monkeypatch.setattr(jira_cloud, "_fetch_issue_json", fake_fetch_issue_json) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-9999-ignored")) + assert result.status == "fetched" + assert result.ticket is not None + + def test_explicit_bearer_auth_takes_precedence_over_implicit_scoped_detection(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net") + monkeypatch.setenv("JIRA_API_EMAIL", "user@example.com") + monkeypatch.setenv("JIRA_API_TOKEN", "token") + monkeypatch.setenv("JIRA_AUTH_TYPE", "bearer") + monkeypatch.setenv("JIRA_CLOUD_ID", "2672de8d-bce7-4cb5-9e65-128f96b0bd9a") + + async def fake_fetch_issue_json(session, auth, key): + assert auth.auth_mode == "bearer" + assert auth.api_base_url == "https://api.atlassian.com/ex/jira/2672de8d-bce7-4cb5-9e65-128f96b0bd9a" + return { + "fields": { + "summary": "Implement Jira support", + "labels": [], + "subtasks": [], + }, + "renderedFields": {"description": "

Hello world

"}, + } + + monkeypatch.setattr(jira_cloud, "_fetch_issue_json", fake_fetch_issue_json) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-9999-ignored")) + assert result.status == "fetched" + assert result.ticket is not None + + def test_fetch_jira_ticket_context_returns_note_when_scoped_token_missing_cloud_id(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net") + monkeypatch.setenv("JIRA_API_EMAIL", "user@example.com") + monkeypatch.setenv("JIRA_API_TOKEN", "token") + monkeypatch.setenv("JIRA_AUTH_TYPE", "scoped") + monkeypatch.delenv("JIRA_CLOUD_ID", raising=False) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-1234-new")) + assert result.status == "missing_config" + assert result.key == "OPS-1234" + assert result.ticket is None + assert "configuration is missing" in result.note + + def test_fetch_jira_ticket_context_returns_note_when_scoped_token_missing_email(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net") + monkeypatch.delenv("JIRA_API_EMAIL", raising=False) + monkeypatch.setenv("JIRA_API_TOKEN", "token") + monkeypatch.setenv("JIRA_AUTH_TYPE", "scoped") + monkeypatch.setenv("JIRA_CLOUD_ID", "2672de8d-bce7-4cb5-9e65-128f96b0bd9a") + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-1234-new")) + assert result.status == "missing_config" + assert result.key == "OPS-1234" + assert result.ticket is None + assert "configuration is missing" in result.note + + def test_try_fetch_jira_ticket_returns_none_without_env(self, monkeypatch): + monkeypatch.delenv("JIRA_BASE_URL", raising=False) + monkeypatch.delenv("JIRA_API_EMAIL", raising=False) + monkeypatch.delenv("JIRA_API_TOKEN", raising=False) + monkeypatch.delenv("JIRA_AUTH_TYPE", raising=False) + monkeypatch.delenv("JIRA_CLOUD_ID", raising=False) + + ticket = asyncio.run(jira_cloud.try_fetch_jira_ticket("OPS-1234 new feature", "OPS-1234-new")) + assert ticket is None + + def test_fetch_jira_ticket_context_returns_note_when_ticket_missing(self, monkeypatch): + monkeypatch.setenv("JIRA_BASE_URL", "https://wkdauto.atlassian.net") + monkeypatch.setenv("JIRA_API_EMAIL", "user@example.com") + monkeypatch.setenv("JIRA_API_TOKEN", "token") + + async def fake_fetch_issue_json(session, auth, key): + return None + + monkeypatch.setattr(jira_cloud, "_fetch_issue_json", fake_fetch_issue_json) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-9999-ignored")) + assert result.status == "not_found" + assert result.key == "OPS-1234" + assert result.ticket is None + assert "OPS-1234" in result.note + assert "ticket compliance analysis was skipped" in result.note + + def test_fetch_jira_ticket_context_returns_note_when_env_missing(self, monkeypatch): + monkeypatch.delenv("JIRA_BASE_URL", raising=False) + monkeypatch.delenv("JIRA_API_EMAIL", raising=False) + monkeypatch.setenv("JIRA_API_TOKEN", "token") + monkeypatch.delenv("JIRA_AUTH_TYPE", raising=False) + monkeypatch.delenv("JIRA_CLOUD_ID", raising=False) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-1234-new")) + assert result.status == "missing_config" + assert result.key == "OPS-1234" + assert result.ticket is None + assert "configuration is missing" in result.note + + def test_fetch_jira_ticket_context_returns_no_note_when_jira_not_configured(self, monkeypatch): + monkeypatch.delenv("JIRA_BASE_URL", raising=False) + monkeypatch.delenv("JIRA_API_EMAIL", raising=False) + monkeypatch.delenv("JIRA_API_TOKEN", raising=False) + monkeypatch.delenv("JIRA_AUTH_TYPE", raising=False) + monkeypatch.delenv("JIRA_CLOUD_ID", raising=False) + + result = asyncio.run(jira_cloud.fetch_jira_ticket_context("OPS-1234 new feature", "OPS-1234-new")) + assert result.status == "not_configured" + assert result.key == "OPS-1234" + assert result.ticket is None + assert result.note == "" diff --git a/tests/unittest/test_pr_reviewer_ticket_note.py b/tests/unittest/test_pr_reviewer_ticket_note.py new file mode 100644 index 0000000000..81aebc1d4a --- /dev/null +++ b/tests/unittest/test_pr_reviewer_ticket_note.py @@ -0,0 +1,41 @@ +from pr_agent.tools.pr_reviewer import inject_ticket_compliance_note + + +def test_inject_ticket_compliance_note_gfm(): + markdown = "## PR Reviewer Guide 🔍\n\nHere are some key observations to aid the review process:\n\n\n\n
existing row
\n" + note = "Jira ticket `COR-5953` was detected in the PR title or branch, but the Jira ticket was not found, so ticket compliance analysis was skipped." + + result = inject_ticket_compliance_note(markdown, note, gfm_supported=True) + + assert "**🎫 Ticket compliance status**" in result + assert "COR-5953" in result + assert result.index("Ticket compliance status") < result.index("existing row") + + +def test_inject_ticket_compliance_note_non_gfm(): + markdown = "## PR Reviewer Guide 🔍\n\nExisting content" + note = "Jira ticket `COR-5953` was detected in the PR title or branch, but the Jira ticket was not found, so ticket compliance analysis was skipped." + + result = inject_ticket_compliance_note(markdown, note, gfm_supported=False) + + assert "### 🎫 Ticket compliance status" in result + assert "COR-5953" in result + assert result.index("Ticket compliance status") < result.index("Existing content") + + +def test_inject_ticket_compliance_note_with_empty_markdown(): + note = "Jira ticket `COR-5953` was detected in the PR title or branch, but the Jira ticket was not found, so ticket compliance analysis was skipped." + + result = inject_ticket_compliance_note("", note, gfm_supported=False) + + assert result.startswith("### 🎫 Ticket compliance status") + assert "COR-5953" in result + + +def test_inject_ticket_compliance_note_with_none_markdown(): + note = "Jira ticket `COR-5953` was detected in the PR title or branch, but the Jira ticket was not found, so ticket compliance analysis was skipped." + + result = inject_ticket_compliance_note(None, note, gfm_supported=True) + + assert "**🎫 Ticket compliance status**" in result + assert "COR-5953" in result