diff --git a/pr_agent/settings/.secrets_template.toml b/pr_agent/settings/.secrets_template.toml index e0d8e11d10..bb493f098c 100644 --- a/pr_agent/settings/.secrets_template.toml +++ b/pr_agent/settings/.secrets_template.toml @@ -109,6 +109,12 @@ url = "" org = "" pat = "" +[jira] +# For fetching ticket context from Jira (Cloud or Server/Data Center) +jira_base_url = "" # e.g. https://your-org.atlassian.net +jira_api_email = "" # Atlassian account email (Cloud) or username (Server basic auth) +jira_api_token = "" # API token (Cloud), password (Server basic auth), or PAT (Server) + [azure_devops_server] # For Azure devops Server basic auth - configured in the webhook creation # Optional, uncomment if you want to use Azure devops webhooks. Value assigned when you create the webhook diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index f4d63a73f2..6bea54bed8 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -318,6 +318,17 @@ pr_commands = [ "/improve --pr_code_suggestions.commitable_code_suggestions=true", ] +[jira] +# Credentials for fetching ticket context from Jira (Cloud or Server/Data Center). +# Leave empty to disable Jira ticket lookup. Set these via environment variables or +# the secrets file rather than committing them (e.g. jira__jira_api_token). +jira_base_url = "" # e.g. https://your-org.atlassian.net (required for Server/PAT and shortened keys) +jira_api_email = "" # Atlassian account email (Cloud), or username (Server basic auth) +jira_api_token = "" # API token (Cloud), password (Server basic auth), or PAT (Server) +# Custom field id holding acceptance criteria / requirements, mapped to the ticket +# "requirements" section. Instance-specific (e.g. "customfield_10127"); empty disables it. +jira_requirements_field = "" + [litellm] # use_client = false # drop_params = false diff --git a/pr_agent/tools/ticket_pr_compliance_check.py b/pr_agent/tools/ticket_pr_compliance_check.py index 6d25d76b19..8155ac060e 100644 --- a/pr_agent/tools/ticket_pr_compliance_check.py +++ b/pr_agent/tools/ticket_pr_compliance_check.py @@ -1,6 +1,8 @@ import re import traceback +from atlassian import Jira + from pr_agent.config_loader import get_settings from pr_agent.git_providers import GithubProvider from pr_agent.git_providers import AzureDevopsProvider @@ -13,8 +15,14 @@ # Option A: issue number at start of branch or after /, followed by - or end (e.g. feature/1-test-issue, 123-fix) BRANCH_ISSUE_PATTERN = re.compile(r"(?:^|/)(\d{1,6})(?=-|$)") +# Max number of tickets to analyse per PR, and max characters of ticket body to keep. +MAX_TICKETS = 3 +MAX_TICKET_CHARACTERS = 10000 + def find_jira_tickets(text): - # Regular expression patterns for JIRA tickets + # Regular expression patterns for JIRA tickets. Matching is case-insensitive so + # lowercased branch names (e.g. bugfix/abc-123-description) are detected; keys are + # normalized to upper case to match Jira's canonical form. patterns = [ r'\b[A-Z]{2,10}-\d{1,7}\b', # Standard JIRA ticket format (e.g., PROJ-123) r'(?:https?://[^\s/]+/browse/)?([A-Z]{2,10}-\d{1,7})\b' # JIRA URL or just the ticket @@ -22,7 +30,7 @@ def find_jira_tickets(text): tickets = set() for pattern in patterns: - matches = re.findall(pattern, text) + matches = re.findall(pattern, text, flags=re.IGNORECASE) for match in matches: if isinstance(match, tuple): # If it's a tuple (from the URL pattern), take the last non-empty group @@ -30,11 +38,121 @@ def find_jira_tickets(text): else: ticket = match if ticket: - tickets.add(ticket) + tickets.add(ticket.upper()) return list(tickets) +def _get_jira_client(): + """ + Build a Jira client from the [jira] settings. Returns None if Jira is not configured. + Cloud uses email + API token; Server/Data Center uses username + password, or a PAT + (passed as the token) together with a base url. + """ + base_url = get_settings().get("JIRA.JIRA_BASE_URL", None) + api_email = get_settings().get("JIRA.JIRA_API_EMAIL", None) + api_token = get_settings().get("JIRA.JIRA_API_TOKEN", None) + if not (base_url and api_token): + return None + try: + if api_email: + return Jira(url=base_url.rstrip("/"), username=api_email, password=api_token) + # No email/username: treat the token as a Server/Data Center PAT. + return Jira(url=base_url.rstrip("/"), token=api_token) + except Exception as e: + get_logger().error(f"Failed to initialize Jira client: {e}", + artifact={"traceback": traceback.format_exc()}) + return None + + +def extract_jira_tickets(text, max_characters=MAX_TICKET_CHARACTERS): + """ + Find Jira ticket keys in the given text and fetch their content. Returns a list of + ticket dicts in the same shape used by the rest of the ticket-analysis flow. Returns + an empty list when Jira is not configured or no keys are found. + """ + jira_client = _get_jira_client() + if jira_client is None: + return [] + + base_url = get_settings().get("JIRA.JIRA_BASE_URL", "").rstrip("/") + # Custom field that holds acceptance criteria / requirements. The field id is + # instance-specific (e.g. "customfield_10127"), so it must be configured; empty + # means no requirements are extracted. + requirements_field = get_settings().get("JIRA.JIRA_REQUIREMENTS_FIELD", "") or "" + keys = find_jira_tickets(text or "") + if len(keys) > MAX_TICKETS: + get_logger().info(f"Too many Jira tickets found: {len(keys)}; limiting to {MAX_TICKETS}") + keys = keys[:MAX_TICKETS] + + tickets_content = [] + for key in keys: + try: + issue = jira_client.issue(key) + except Exception as e: + get_logger().warning(f"Failed to fetch Jira ticket {key}: {e}") + continue + if not issue: + continue + + fields = issue.get("fields", {}) or {} + body = fields.get("description") or "" + if not isinstance(body, str): + body = "" + if len(body) > max_characters: + body = body[:max_characters] + "..." + + requirements = "" + if requirements_field: + requirements = fields.get(requirements_field) or "" + if not isinstance(requirements, str): + requirements = "" + + labels = fields.get("labels", []) or [] + tickets_content.append({ + "ticket_id": key, + "ticket_url": f"{base_url}/browse/{key}" if base_url else "", + "title": fields.get("summary", ""), + "body": body, + "requirements": requirements, + "labels": ", ".join(labels), + }) + return tickets_content + + +def _get_pr_title(git_provider) -> str: + """Return the PR/MR title across providers (GitHub/Bitbucket use .pr, GitLab .mr).""" + for attr in ("pr", "mr"): + obj = getattr(git_provider, attr, None) + title = getattr(obj, "title", None) + if title: + return title + return "" + + +def add_jira_tickets(git_provider, tickets_content): + """ + Provider-agnostic Jira lookup. Scans the PR title, description and branch name for + Jira ticket keys and appends any found tickets to tickets_content (de-duplicated by + ticket_url). No-op when Jira is not configured. Works for any git provider, since it + only relies on get_user_description() and get_pr_branch(). + """ + try: + jira_context = "\n".join(filter(None, [ + _get_pr_title(git_provider), + git_provider.get_user_description() or "", + git_provider.get_pr_branch() or "", + ])) + existing_urls = {t.get("ticket_url") for t in tickets_content} + for jira_ticket in extract_jira_tickets(jira_context, MAX_TICKET_CHARACTERS): + if jira_ticket.get("ticket_url") not in existing_urls: + tickets_content.append(jira_ticket) + except Exception as e: + get_logger().error(f"Error extracting Jira tickets: {e}", + artifact={"traceback": traceback.format_exc()}) + return tickets_content + + def extract_ticket_links_from_pr_description(pr_description, repo_path, base_url_html='https://github.com'): """ Extract all ticket links from PR description @@ -55,10 +173,10 @@ def extract_ticket_links_from_pr_description(pr_description, repo_path, base_url if issue_number.isdigit() and len(issue_number) < 5 and repo_path: github_tickets.add(f'{base_url_html.strip("/")}/{repo_path}/issues/{issue_number}') - if len(github_tickets) > 3: + if len(github_tickets) > MAX_TICKETS: get_logger().info(f"Too many tickets found in PR description: {len(github_tickets)}") - # Limit the number of tickets to 3 - github_tickets = set(list(github_tickets)[:3]) + # Limit the number of tickets + github_tickets = set(list(github_tickets)[:MAX_TICKETS]) except Exception as e: get_logger().error(f"Error extracting tickets error= {e}", artifact={"traceback": traceback.format_exc()}) @@ -106,8 +224,9 @@ def extract_ticket_links_from_branch_name(branch_name, repo_path, base_url_html= async def extract_tickets(git_provider): - MAX_TICKET_CHARACTERS = 10000 try: + tickets_content = [] + if isinstance(git_provider, GithubProvider): user_description = git_provider.get_user_description() description_tickets = extract_ticket_links_from_pr_description( @@ -123,12 +242,11 @@ async def extract_tickets(git_provider): if link not in seen: seen.add(link) merged.append(link) - if len(merged) > 3: + if len(merged) > MAX_TICKETS: get_logger().info(f"Too many tickets (description + branch): {len(merged)}") - tickets = merged[:3] + tickets = merged[:MAX_TICKETS] else: tickets = merged - tickets_content = [] if tickets: @@ -188,11 +306,8 @@ async def extract_tickets(git_provider): 'sub_issues': sub_issues_content # Store sub-issues content }) - return tickets_content - 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", "") @@ -214,7 +329,13 @@ async def extract_tickets(git_provider): f"Error processing Azure DevOps ticket: {e}", artifact={"traceback": traceback.format_exc()}, ) - return tickets_content + + # Provider-agnostic Jira lookup. Tickets are often referenced by key in the PR + # title, description or branch name rather than via a provider-native link, so + # this runs for every provider and is a no-op when Jira is not configured. + add_jira_tickets(git_provider, tickets_content) + + return tickets_content except Exception as e: get_logger().error(f"Error extracting tickets error= {e}", diff --git a/tests/unittest/test_jira_ticket_extraction.py b/tests/unittest/test_jira_ticket_extraction.py new file mode 100644 index 0000000000..554f59deac --- /dev/null +++ b/tests/unittest/test_jira_ticket_extraction.py @@ -0,0 +1,231 @@ +from unittest.mock import MagicMock, patch + +from pr_agent.config_loader import get_settings +from pr_agent.tools.ticket_pr_compliance_check import ( + _get_pr_title, + add_jira_tickets, + extract_jira_tickets, + find_jira_tickets, +) + + +class TestFindJiraTickets: + """Jira key extraction from arbitrary text (PR title, description, branch name).""" + + def test_uppercase_key_with_prefix(self): + """feature/ABC-123-description -> ABC-123""" + assert find_jira_tickets("feature/ABC-123-description-of-branch") == ["ABC-123"] + + def test_lowercase_key_with_prefix_normalized(self): + """bugfix/abc-123-description -> ABC-123 (case-insensitive, normalized to upper)""" + assert find_jira_tickets("bugfix/abc-123-description-of-branch") == ["ABC-123"] + + def test_mixed_case_key_normalized(self): + """Abc-123 -> ABC-123""" + assert find_jira_tickets("Abc-123-fix") == ["ABC-123"] + + def test_arbitrary_prefix_segment(self): + """Any prefix segment, not just feature/bugfix.""" + assert find_jira_tickets("chore/PROJ-45-cleanup") == ["PROJ-45"] + assert find_jira_tickets("hotfix/proj-45-cleanup") == ["PROJ-45"] + + def test_key_at_start_no_prefix(self): + """ABC-123-fix -> ABC-123""" + assert find_jira_tickets("ABC-123-fix") == ["ABC-123"] + + def test_key_anywhere_in_branch(self): + """Key embedded in the middle of a branch name is still found.""" + assert find_jira_tickets("release/v1.2.3-ABC-9-final") == ["ABC-9"] + + def test_key_in_description_text(self): + """Key mentioned in free text.""" + assert find_jira_tickets("This implements ABC-123 as discussed") == ["ABC-123"] + + def test_browse_url(self): + """Full Jira browse URL -> key.""" + assert find_jira_tickets( + "see https://acme.atlassian.net/browse/ABC-123 for details" + ) == ["ABC-123"] + + def test_no_ticket(self): + """Branch with no key -> []""" + assert find_jira_tickets("feature/no-ticket-here") == [] + assert find_jira_tickets("") == [] + + def test_multiple_tickets_deduped_in_order(self): + """Multiple distinct keys are all returned, de-duplicated and case-normalized.""" + result = find_jira_tickets("ABC-1 and DEF-2, again ABC-1 and abc-1") + assert set(result) == {"ABC-1", "DEF-2"} + + +class TestExtractJiraTickets: + """End-to-end extraction: find keys, fetch via the Jira client, map to ticket dicts.""" + + def _configure_jira(self): + get_settings().set("JIRA.JIRA_BASE_URL", "https://acme.atlassian.net") + get_settings().set("JIRA.JIRA_API_EMAIL", "me@acme.com") + get_settings().set("JIRA.JIRA_API_TOKEN", "token123") + + def _disable_jira(self): + get_settings().set("JIRA.JIRA_BASE_URL", "") + get_settings().set("JIRA.JIRA_API_EMAIL", "") + get_settings().set("JIRA.JIRA_API_TOKEN", "") + + def _fake_client(self, fields=None): + fields = fields or {"summary": "Title", "description": "Body", "labels": ["a"]} + client = MagicMock() + client.issue.return_value = {"fields": fields} + return client + + def test_returns_empty_when_not_configured(self): + self._disable_jira() + assert extract_jira_tickets("bugfix/abc-123-x") == [] + + def test_fetches_lowercase_branch_key(self): + """The whole point: a lowercased branch key is detected and fetched as upper.""" + self._configure_jira() + client = self._fake_client() + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + result = extract_jira_tickets("bugfix/abc-123-description-of-branch") + client.issue.assert_called_once_with("ABC-123") + assert len(result) == 1 + assert result[0]["ticket_id"] == "ABC-123" + assert result[0]["ticket_url"] == "https://acme.atlassian.net/browse/ABC-123" + assert result[0]["title"] == "Title" + assert result[0]["labels"] == "a" + + def test_requirements_field_populated_when_configured(self): + """When jira_requirements_field is set, that custom field maps to requirements.""" + self._configure_jira() + get_settings().set("JIRA.JIRA_REQUIREMENTS_FIELD", "customfield_10127") + client = MagicMock() + client.issue.return_value = {"fields": { + "summary": "T", "description": "B", "labels": [], + "customfield_10127": "Acceptance criteria text", + }} + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + result = extract_jira_tickets("ABC-1") + assert result[0]["requirements"] == "Acceptance criteria text" + + def test_requirements_empty_when_field_not_configured(self): + """With no requirements field configured, requirements stays empty.""" + self._configure_jira() + get_settings().set("JIRA.JIRA_REQUIREMENTS_FIELD", "") + client = self._fake_client() + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + result = extract_jira_tickets("ABC-1") + assert result[0]["requirements"] == "" + + def test_checks_all_tickets_when_multiple(self): + """When several distinct keys are present, each is fetched.""" + self._configure_jira() + client = self._fake_client() + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + result = extract_jira_tickets("ABC-1 DEF-2 GHI-3") + fetched = {c.args[0] for c in client.issue.call_args_list} + assert fetched == {"ABC-1", "DEF-2", "GHI-3"} + assert len(result) == 3 + + def test_caps_candidate_keys_at_three(self): + """No more than three candidate keys are fetched (matches the GitHub branch).""" + self._configure_jira() + client = self._fake_client() + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + result = extract_jira_tickets("ABC-1 ABC-2 ABC-3 ABC-4 ABC-5") + assert client.issue.call_count == 3 + assert len(result) == 3 + + def test_skips_ticket_on_fetch_error(self): + """A failed fetch for one key does not abort the others.""" + self._configure_jira() + client = MagicMock() + client.issue.side_effect = [ + Exception("404 not found"), + {"fields": {"summary": "Second", "description": "B", "labels": []}}, + ] + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + result = extract_jira_tickets("ABC-1 ABC-2") + assert len(result) == 1 + assert result[0]["title"] == "Second" + + def test_nonexistent_keys_are_skipped(self): + """Key-like noise (utf-8, sha-1) that does not resolve in Jira is skipped, + leaving only the real ticket.""" + self._configure_jira() + + def fake_issue(key): + if key == "ABC-123": + return {"fields": {"summary": "Real", "description": "Body", "labels": []}} + raise Exception("404 not found") + + client = MagicMock() + client.issue.side_effect = fake_issue + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + result = extract_jira_tickets("abc-123 utf-8") + assert [t["ticket_id"] for t in result] == ["ABC-123"] + + +class TestGetPrTitle: + """Provider-agnostic title access (GitHub/Bitbucket use .pr, GitLab uses .mr).""" + + def test_reads_pr_title(self): + gp = MagicMock(spec=["pr"]) + gp.pr = MagicMock(title="From PR object") + assert _get_pr_title(gp) == "From PR object" + + def test_reads_mr_title_when_no_pr(self): + """GitLab stores the merge request as .mr, not .pr.""" + gp = MagicMock(spec=["mr"]) + gp.mr = MagicMock(title="From MR object") + assert _get_pr_title(gp) == "From MR object" + + def test_returns_empty_when_no_title(self): + gp = MagicMock(spec=[]) + assert _get_pr_title(gp) == "" + + +class TestAddJiraTickets: + """Provider-agnostic Jira append used by extract_tickets for every provider.""" + + def _provider(self, title="", description="", branch=""): + gp = MagicMock(spec=["pr", "get_user_description", "get_pr_branch"]) + gp.pr = MagicMock(title=title) + gp.get_user_description.return_value = description + gp.get_pr_branch.return_value = branch + return gp + + def _configure_jira(self): + get_settings().set("JIRA.JIRA_BASE_URL", "https://acme.atlassian.net") + get_settings().set("JIRA.JIRA_API_EMAIL", "me@acme.com") + get_settings().set("JIRA.JIRA_API_TOKEN", "token123") + get_settings().set("JIRA.JIRA_REQUIREMENTS_FIELD", "") + + def test_appends_ticket_from_any_provider(self): + """Works off get_user_description + get_pr_branch, so it is provider-neutral.""" + self._configure_jira() + client = MagicMock() + client.issue.return_value = {"fields": {"summary": "T", "description": "B", "labels": []}} + gp = self._provider(branch="feature/ABC-123-x") + out = [] + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + add_jira_tickets(gp, out) + assert [t["ticket_id"] for t in out] == ["ABC-123"] + + def test_dedupes_against_existing_tickets(self): + """A Jira ticket already present (same url) is not added twice.""" + self._configure_jira() + client = MagicMock() + client.issue.return_value = {"fields": {"summary": "T", "description": "B", "labels": []}} + gp = self._provider(title="ABC-123") + existing = [{"ticket_url": "https://acme.atlassian.net/browse/ABC-123"}] + with patch("pr_agent.tools.ticket_pr_compliance_check.Jira", return_value=client): + add_jira_tickets(gp, existing) + assert len(existing) == 1 + + def test_noop_when_jira_not_configured(self): + get_settings().set("JIRA.JIRA_BASE_URL", "") + get_settings().set("JIRA.JIRA_API_TOKEN", "") + gp = self._provider(branch="feature/ABC-123-x") + out = [] + add_jira_tickets(gp, out) + assert out == []