Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions chat/sippy_agent/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,12 @@ async def _create_tools(self) -> List[BaseTool]:
SippyTestDetailsTool(sippy_api_url=self.config.sippy_api_url),
SippyJiraIncidentTool(
jira_url=self.config.jira_url,
jira_username=self.config.jira_username,
jira_token=self.config.jira_token,
jira_basic_auth_token=self.config.jira_basic_auth_token,
),
SippyJiraIssueTool(
jira_url=self.config.jira_url,
jira_basic_auth_token=self.config.jira_basic_auth_token,
),
Comment on lines 186 to 193
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Disable Jira tools when the cloud credential is missing.

jira_basic_auth_token is optional, but these tools are now always advertised. If the companion rollout or a local env has not provided JIRA_BASIC_AUTH_TOKEN yet, the agent will still call them and surface 401/403s to users. Prefer skipping registration until the credential exists, or fail startup explicitly if Jira support is mandatory.

Suggested change
         tools = [
             SippyProwJobSummaryTool(sippy_api_url=self.config.sippy_api_url),
             SippyProwJobPayloadTool(sippy_api_url=self.config.sippy_api_url),
             SippyLogAnalyzerTool(sippy_api_url=self.config.sippy_api_url),
             SippyTestDetailsTool(sippy_api_url=self.config.sippy_api_url),
-            SippyJiraIncidentTool(
-                jira_url=self.config.jira_url,
-                jira_basic_auth_token=self.config.jira_basic_auth_token,
-            ),
-            SippyJiraIssueTool(
-                jira_url=self.config.jira_url,
-                jira_basic_auth_token=self.config.jira_basic_auth_token,
-            ),
             TriagePotentialMatchesTool(sippy_api_url=self.config.sippy_api_url),
             SippyReleasePayloadTool(),
             SippyPayloadDetailsTool(),
             JUnitParserTool(),
             AggregatedJobAnalyzerTool(sippy_api_url=self.config.sippy_api_url),
             AggregatedYAMLParserTool(),
         ]
+
+        if self.config.jira_basic_auth_token:
+            tools.extend([
+                SippyJiraIncidentTool(
+                    jira_url=self.config.jira_url,
+                    jira_basic_auth_token=self.config.jira_basic_auth_token,
+                ),
+                SippyJiraIssueTool(
+                    jira_url=self.config.jira_url,
+                    jira_basic_auth_token=self.config.jira_basic_auth_token,
+                ),
+            ])
+        else:
+            logger.warning("Jira tools disabled: JIRA_BASIC_AUTH_TOKEN not configured")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
SippyJiraIncidentTool(
jira_url=self.config.jira_url,
jira_username=self.config.jira_username,
jira_token=self.config.jira_token,
jira_basic_auth_token=self.config.jira_basic_auth_token,
),
SippyJiraIssueTool(
jira_url=self.config.jira_url,
jira_basic_auth_token=self.config.jira_basic_auth_token,
),
]
if self.config.jira_basic_auth_token:
tools.extend([
SippyJiraIncidentTool(
jira_url=self.config.jira_url,
jira_basic_auth_token=self.config.jira_basic_auth_token,
),
SippyJiraIssueTool(
jira_url=self.config.jira_url,
jira_basic_auth_token=self.config.jira_basic_auth_token,
),
])
else:
logger.warning("Jira tools disabled: JIRA_BASIC_AUTH_TOKEN not configured")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@chat/sippy_agent/agent.py` around lines 186 - 193, The Jira tool instances
SippyJiraIncidentTool and SippyJiraIssueTool are being registered
unconditionally even when self.config.jira_basic_auth_token is missing; update
the tool registration in agent.py to check self.config.jira_basic_auth_token
before creating/adding these tools and only register them when the token is
present, or alternatively add a config flag (e.g., self.config.require_jira) and
raise a clear startup error if require_jira is true but jira_basic_auth_token is
absent; make the change around the code that constructs SippyJiraIncidentTool
and SippyJiraIssueTool so they are skipped when the credential is not provided.

SippyJiraIssueTool(jira_url=self.config.jira_url),
TriagePotentialMatchesTool(sippy_api_url=self.config.sippy_api_url),
SippyReleasePayloadTool(),
SippyPayloadDetailsTool(),
Expand Down
11 changes: 4 additions & 7 deletions chat/sippy_agent/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,11 @@ class Config(BaseModel):
sippy_api_url: Optional[str] = Field(default_factory=lambda: os.getenv("SIPPY_API_URL"), description="Base URL for the Sippy API")

# Jira Configuration
jira_url: str = Field(default_factory=lambda: os.getenv("JIRA_URL", "https://issues.redhat.com"), description="Jira instance URL")
jira_url: str = Field(default_factory=lambda: os.getenv("JIRA_URL", "https://redhat.atlassian.net"), description="Jira instance URL")

jira_username: Optional[str] = Field(
default_factory=lambda: os.getenv("JIRA_USERNAME"), description="Jira username for authentication (optional for public queries)"
)

jira_token: Optional[str] = Field(
default_factory=lambda: os.getenv("JIRA_TOKEN"), description="Jira API token for authentication (optional for public queries)"
jira_basic_auth_token: Optional[str] = Field(
default_factory=lambda: os.getenv("JIRA_BASIC_AUTH_TOKEN"),
description="Jira basic auth token in the format '[email protected]:api_token' for Atlassian Cloud authentication",
)

# MCP Configuration
Expand Down
35 changes: 20 additions & 15 deletions chat/sippy_agent/tools/jira_incidents.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Tool for querying Jira for known open incidents in the TRT project.
"""

import base64
import json
import logging
from typing import Any, Dict, Optional, Type
Expand All @@ -20,15 +21,22 @@ class SippyJiraIncidentTool(SippyBaseTool):
description: str = "Get a JSON object with a list of all known open TRT incidents from Jira."

# Add Jira configuration as proper fields
jira_url: str = Field(default="https://issues.redhat.com", description="Jira instance URL")
jira_username: Optional[str] = Field(default=None, description="Jira username")
jira_token: Optional[str] = Field(default=None, description="Jira API token")
jira_url: str = Field(default="https://redhat.atlassian.net", description="Jira instance URL")
jira_basic_auth_token: Optional[str] = Field(default=None, description="Jira basic auth token (user:api_token)")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left JIRA_TOKEN support in place for when we move over to a SA. Would it be worth keeping that support in place here and having the code detect which envar was set or do you prefer another update when we switch over some time in the future.


class JiraIncidentInput(SippyToolInput):
jira_url: Optional[str] = Field(default=None, description="Jira URL (optional, uses config if not provided)")

args_schema: Type[SippyToolInput] = JiraIncidentInput

def _get_auth_headers(self) -> Dict[str, str]:
"""Build authentication headers for Jira API requests."""
headers = {"Accept": "application/json", "Content-Type": "application/json"}
if self.jira_basic_auth_token:
encoded = base64.b64encode(self.jira_basic_auth_token.encode()).decode()
headers["Authorization"] = f"Basic {encoded}"
return headers

def _run(self, jira_url: Optional[str] = None) -> Dict[str, Any]:
"""Query Jira for known open incidents."""
# Use provided URL or fall back to instance URL
Expand All @@ -37,33 +45,30 @@ def _run(self, jira_url: Optional[str] = None) -> Dict[str, Any]:
if not api_url:
return {"error": "No Jira URL configured. Please set JIRA_URL environment variable or provide jira_url parameter."}

# Construct the Jira REST API endpoint
endpoint = f"{api_url.rstrip('/')}/rest/api/2/search"
# Use the v3 POST-based search endpoint
endpoint = f"{api_url.rstrip('/')}/rest/api/3/search/jql"

# Build JQL query for TRT project incidents
jql_parts = ['project = "TRT"', 'labels = "trt-incident"', "status not in (Closed, Done, Resolved)"]

jql = " AND ".join(jql_parts)

try:
# Prepare request parameters
params = {
# Prepare request body
body = {
"jql": jql,
"fields": "key,summary,status,priority,created,updated,description,labels",
"maxResults": 20, # Limit results
"fields": ["key", "summary", "status", "priority", "created", "updated", "description", "labels"],
"maxResults": 20,
}

# Prepare authentication if available
auth = None
if self.jira_username and self.jira_token:
auth = (self.jira_username, self.jira_token)
headers = self._get_auth_headers()

logger.info("Querying Jira for all open TRT incidents")
logger.info(f"JQL: {jql}")

# Make the API request
with httpx.Client(timeout=30.0) as client:
response = client.get(endpoint, params=params, auth=auth, headers={"Accept": "application/json"})
response = client.post(endpoint, json=body, headers=headers)
response.raise_for_status()

data = response.json()
Expand All @@ -81,7 +86,7 @@ def _run(self, jira_url: Optional[str] = None) -> Dict[str, Any]:
except httpx.HTTPStatusError as e:
logger.error(f"HTTP error querying Jira: {e}")
if e.response.status_code == 401:
return {"error": "Jira authentication failed. Check JIRA_USERNAME and JIRA_TOKEN environment variables."}
return {"error": "Jira authentication failed. Check JIRA_BASIC_AUTH_TOKEN environment variable."}
elif e.response.status_code == 403:
return {"error": "Access denied to Jira. You may need authentication or permissions to view TRT project."}
else:
Expand Down
83 changes: 60 additions & 23 deletions chat/sippy_agent/tools/jira_issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
Tool for analyzing Jira issues and their comments.
"""

import base64
import json
import logging
from typing import Any, Dict, Type
from typing import Any, Dict, Optional, Type
from pydantic import Field
import httpx
from datetime import datetime, timezone
Expand All @@ -19,7 +20,7 @@ class SippyJiraIssueTool(SippyBaseTool):

name: str = "get_jira_issue_analysis"
description: str = """Get Jira issue information including description, status, and recent comments.

This tool provides:
- Issue description and current status
- Recent comments (sorted newest first)
Expand All @@ -28,12 +29,21 @@ class SippyJiraIssueTool(SippyBaseTool):
Input: issue_key (the Jira issue key, e.g., OCPBUGS-12345)"""

jira_url: str = Field(description="Jira base URL")
jira_basic_auth_token: Optional[str] = Field(default=None, description="Jira basic auth token (user:api_token)")

class JiraIssueInput(SippyToolInput):
issue_key: str = Field(description="Jira issue key (e.g., OCPBUGS-12345)")

args_schema: Type[SippyToolInput] = JiraIssueInput

def _get_auth_headers(self) -> Dict[str, str]:
"""Build authentication headers for Jira API requests."""
headers = {"Accept": "application/json"}
if self.jira_basic_auth_token:
encoded = base64.b64encode(self.jira_basic_auth_token.encode()).decode()
headers["Authorization"] = f"Basic {encoded}"
return headers

def _run(self, *args, **kwargs: Any) -> Dict[str, Any]:
"""Get Jira issue analysis."""

Expand All @@ -48,26 +58,28 @@ def _run(self, *args, **kwargs: Any) -> Dict[str, Any]:
issue_key = args.issue_key.strip()

try:
# Make API request to get issue details using configured base URL
api_url = f"{self.jira_url.rstrip('/')}/rest/api/2/issue/{issue_key}"
# Make API request to get issue details using v3 API
api_url = f"{self.jira_url.rstrip('/')}/rest/api/3/issue/{issue_key}"

logger.info(f"Making request to {api_url}")


headers = self._get_auth_headers()

with httpx.Client(timeout=30.0) as client:
# Get issue details (no authentication needed for public data)
response = client.get(api_url)
# Get issue details
response = client.get(api_url, headers=headers)
response.raise_for_status()

issue_data = response.json()
# Get comments (no authentication needed for public data)
comments_response = client.get(f"{api_url}/comment")

# Get comments
comments_response = client.get(f"{api_url}/comment", headers=headers)
comments_response.raise_for_status()
comments_data = comments_response.json()

# Process the response
processed_data = self._process_jira_issue(issue_data, comments_data)

return processed_data

except httpx.HTTPStatusError as e:
Expand All @@ -87,14 +99,19 @@ def _run(self, *args, **kwargs: Any) -> Dict[str, Any]:

def _process_jira_issue(self, issue_data: Dict[str, Any], comments_data: Dict[str, Any]) -> Dict[str, Any]:
"""Process Jira issue data and comments to extract key information."""

fields = issue_data.get('fields', {})

# Extract basic issue information
# v3 API returns description as ADF (Atlassian Document Format), convert to plain text
description = fields.get('description')
if isinstance(description, dict):
description = self._adf_to_text(description)

issue_info = {
"key": issue_data.get('key'),
"summary": fields.get('summary'),
"description": fields.get('description'),
"description": description,
"status": fields.get('status', {}).get('name'),
"priority": fields.get('priority', {}).get('name'),
"issue_type": fields.get('issuetype', {}).get('name'),
Expand All @@ -107,28 +124,32 @@ def _process_jira_issue(self, issue_data: Dict[str, Any], comments_data: Dict[st
"labels": fields.get('labels', []),
"components": [c.get('name') for c in fields.get('components', [])],
}

# Process comments
comments = comments_data.get('comments', [])
processed_comments = []

for comment in comments:
body = comment.get('body')
if isinstance(body, dict):
body = self._adf_to_text(body)

comment_info = {
"author": comment.get('author', {}).get('displayName'),
"body": comment.get('body'),
"body": body,
"created": comment.get('created'),
"updated": comment.get('updated'),
}
processed_comments.append(comment_info)

# Sort comments by creation date (newest first)
processed_comments.sort(key=lambda x: x['created'], reverse=True)

# Calculate days since last comment
days_since_last_comment = None
if processed_comments:
days_since_last_comment = self._get_days_old(processed_comments[0]['created'])

return {
"issue": issue_info,
"comments": {
Expand All @@ -138,6 +159,22 @@ def _process_jira_issue(self, issue_data: Dict[str, Any], comments_data: Dict[st
},
}

def _adf_to_text(self, adf: Dict[str, Any]) -> str:
"""Convert Atlassian Document Format to plain text."""
if not isinstance(adf, dict):
return str(adf) if adf else ""

parts = []
for node in adf.get('content', []):
node_type = node.get('type', '')
if node_type == 'text':
parts.append(node.get('text', ''))
elif 'content' in node:
parts.append(self._adf_to_text(node))
elif node_type == 'hardBreak':
parts.append('\n')
return ''.join(parts)
Comment on lines +162 to +176
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Preserve block boundaries when flattening ADF.

_adf_to_text() only emits separators for hardBreak, so paragraph/list/code-block nodes collapse together. Two paragraphs like foo and bar currently become foobar, which changes the content the agent reasons over.

Suggested change
     def _adf_to_text(self, adf: Dict[str, Any]) -> str:
         """Convert Atlassian Document Format to plain text."""
         if not isinstance(adf, dict):
             return str(adf) if adf else ""

-        parts = []
+        parts = []
+        block_nodes = {"paragraph", "heading", "blockquote", "codeBlock", "listItem"}
         for node in adf.get('content', []):
             node_type = node.get('type', '')
             if node_type == 'text':
                 parts.append(node.get('text', ''))
-            elif 'content' in node:
-                parts.append(self._adf_to_text(node))
             elif node_type == 'hardBreak':
                 parts.append('\n')
-        return ''.join(parts)
+            elif 'content' in node:
+                text = self._adf_to_text(node)
+                parts.append(text)
+                if node_type in block_nodes and text and not text.endswith('\n'):
+                    parts.append('\n')
+        return ''.join(parts).rstrip()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def _adf_to_text(self, adf: Dict[str, Any]) -> str:
"""Convert Atlassian Document Format to plain text."""
if not isinstance(adf, dict):
return str(adf) if adf else ""
parts = []
for node in adf.get('content', []):
node_type = node.get('type', '')
if node_type == 'text':
parts.append(node.get('text', ''))
elif 'content' in node:
parts.append(self._adf_to_text(node))
elif node_type == 'hardBreak':
parts.append('\n')
return ''.join(parts)
def _adf_to_text(self, adf: Dict[str, Any]) -> str:
"""Convert Atlassian Document Format to plain text."""
if not isinstance(adf, dict):
return str(adf) if adf else ""
parts = []
block_nodes = {"paragraph", "heading", "blockquote", "codeBlock", "listItem"}
for node in adf.get('content', []):
node_type = node.get('type', '')
if node_type == 'text':
parts.append(node.get('text', ''))
elif node_type == 'hardBreak':
parts.append('\n')
elif 'content' in node:
text = self._adf_to_text(node)
parts.append(text)
if node_type in block_nodes and text and not text.endswith('\n'):
parts.append('\n')
return ''.join(parts).rstrip()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly legitimate concern?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sosiouxme Yes, the concern is legitimate. The ADF format wraps text in block-level nodes (paragraph, heading, listItem, etc.) as siblings in the top-level content array. The current implementation recurses into them but appends no separator, so adjacent paragraphs concatenate directly — "foo" + "bar""foobar". This degrades the text the agent reasons over.

The suggested fix is correct: append \n after each block-level node whose extracted text doesn't already end with one:

def _adf_to_text(self, adf: Dict[str, Any]) -> str:
    """Convert Atlassian Document Format to plain text."""
    if not isinstance(adf, dict):
        return str(adf) if adf else ""

    parts = []
    block_nodes = {"paragraph", "heading", "blockquote", "codeBlock", "listItem"}
    for node in adf.get('content', []):
        node_type = node.get('type', '')
        if node_type == 'text':
            parts.append(node.get('text', ''))
        elif node_type == 'hardBreak':
            parts.append('\n')
        elif 'content' in node:
            text = self._adf_to_text(node)
            parts.append(text)
            if node_type in block_nodes and text and not text.endswith('\n'):
                parts.append('\n')
    return ''.join(parts).rstrip()


def _get_days_old(self, date_string: str) -> int:
"""Calculate how many days old a date string is."""
try:
Expand Down