[Jira]: fix match conditions for kicking off openhands (#12477)

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
Rohit Malhotra
2026-01-20 12:10:03 -08:00
committed by GitHub
parent 275bb689e4
commit 5b149927fb
5 changed files with 480 additions and 72 deletions

View File

@@ -135,52 +135,38 @@ class JiraManager(Manager):
return False, None, None
def parse_webhook(self, payload: Dict) -> JobContext | None:
event_type = payload.get('webhookEvent')
def parse_webhook(self, message: Message) -> JobContext | None:
payload = message.message.get('payload', {})
issue_data = payload.get('issue', {})
issue_id = issue_data.get('id')
issue_key = issue_data.get('key')
self_url = issue_data.get('self', '')
if not self_url:
logger.warning('[Jira] Missing self URL in issue data')
base_api_url = ''
elif '/rest/' in self_url:
base_api_url = self_url.split('/rest/')[0]
else:
# Fallback: extract base URL using urlparse
parsed = urlparse(self_url)
base_api_url = f'{parsed.scheme}://{parsed.netloc}'
if event_type == 'comment_created':
comment = ''
if JiraFactory.is_ticket_comment(message):
comment_data = payload.get('comment', {})
comment = comment_data.get('body', '')
if '@openhands' not in comment:
return None
issue_data = payload.get('issue', {})
issue_id = issue_data.get('id')
issue_key = issue_data.get('key')
base_api_url = issue_data.get('self', '').split('/rest/')[0]
user_data = comment_data.get('author', {})
user_email = user_data.get('emailAddress')
display_name = user_data.get('displayName')
account_id = user_data.get('accountId')
elif event_type == 'jira:issue_updated':
changelog = payload.get('changelog', {})
items = changelog.get('items', [])
labels = [
item.get('toString', '')
for item in items
if item.get('field') == 'labels' and 'toString' in item
]
if 'openhands' not in labels:
return None
issue_data = payload.get('issue', {})
issue_id = issue_data.get('id')
issue_key = issue_data.get('key')
base_api_url = issue_data.get('self', '').split('/rest/')[0]
user_data: dict = comment_data.get('author', {})
elif JiraFactory.is_labeled_ticket(message):
user_data = payload.get('user', {})
user_email = user_data.get('emailAddress')
display_name = user_data.get('displayName')
account_id = user_data.get('accountId')
comment = ''
else:
return None
raise ValueError('Unrecognized jira event')
user_email = user_data.get('emailAddress')
display_name = user_data.get('displayName')
account_id = user_data.get('accountId')
workspace_name = ''
parsedUrl = urlparse(base_api_url)
if parsedUrl.hostname:
workspace_name = parsedUrl.hostname
@@ -209,14 +195,28 @@ class JiraManager(Manager):
base_api_url=base_api_url,
)
async def is_job_requested(self, message: Message) -> bool:
return JiraFactory.is_labeled_ticket(message) or JiraFactory.is_ticket_comment(
message
)
async def receive_message(self, message: Message):
"""Process incoming Jira webhook message."""
payload = message.message.get('payload', {})
job_context = self.parse_webhook(payload)
logger.info('[Jira]: received payload', extra={'payload': payload})
is_job_requested = await self.is_job_requested(message)
if not is_job_requested:
return
job_context = self.parse_webhook(message)
if not job_context:
logger.info('[Jira] Webhook does not match trigger conditions')
logger.info(
'[Jira] Failed to parse webhook payload - missing required fields or invalid structure',
extra={'event_type': payload.get('webhookEvent')},
)
return
# Get workspace by user email domain
@@ -296,12 +296,12 @@ class JiraManager(Manager):
)
return
if not await self.is_job_requested(message, jira_view):
if not await self.is_repository_specified(message, jira_view):
return
await self.start_job(jira_view)
async def is_job_requested(
async def is_repository_specified(
self, message: Message, jira_view: JiraViewInterface
) -> bool:
"""
@@ -330,7 +330,7 @@ class JiraManager(Manager):
return False
except Exception as e:
logger.error(f'[Jira] Error in is_job_requested: {str(e)}')
logger.error(f'[Jira] Error determining repository: {str(e)}')
return False
async def start_job(self, jira_view: JiraViewInterface):

View File

@@ -1,8 +1,8 @@
from dataclasses import dataclass
from integrations.jira.jira_types import JiraViewInterface, StartingConvoException
from integrations.models import JobContext
from integrations.utils import CONVERSATION_URL
from integrations.models import JobContext, Message
from integrations.utils import CONVERSATION_URL, HOST, get_oh_labels, has_exact_mention
from jinja2 import Environment
from storage.jira_conversation import JiraConversation
from storage.jira_integration_store import JiraIntegrationStore
@@ -18,6 +18,8 @@ from openhands.storage.data_models.conversation_metadata import ConversationTrig
integration_store = JiraIntegrationStore.get_instance()
OH_LABEL, INLINE_OH_LABEL = get_oh_labels(HOST)
@dataclass
class JiraNewConversationView(JiraViewInterface):
@@ -99,6 +101,36 @@ class JiraNewConversationView(JiraViewInterface):
class JiraFactory:
"""Factory for creating Jira views based on message content"""
@staticmethod
def is_labeled_ticket(message: Message) -> bool:
payload = message.message.get('payload', {})
event_type = payload.get('webhookEvent')
if event_type != 'jira:issue_updated':
return False
changelog = payload.get('changelog', {})
items = changelog.get('items', [])
labels = [
item.get('toString', '')
for item in items
if item.get('field') == 'labels' and 'toString' in item
]
return OH_LABEL in labels
@staticmethod
def is_ticket_comment(message: Message) -> bool:
payload = message.message.get('payload', {})
event_type = payload.get('webhookEvent')
if event_type != 'comment_created':
return False
comment_data = payload.get('comment', {})
comment_body = comment_data.get('body', '')
return has_exact_mention(comment_body, INLINE_OH_LABEL)
@staticmethod
async def create_jira_view_from_payload(
job_context: JobContext,

View File

@@ -9,11 +9,11 @@ from fastapi import APIRouter, BackgroundTasks, HTTPException, Request, status
from fastapi.responses import JSONResponse, RedirectResponse
from integrations.jira.jira_manager import JiraManager
from integrations.models import Message, SourceType
from integrations.utils import HOST_URL
from pydantic import BaseModel, Field, field_validator
from server.auth.constants import JIRA_CLIENT_ID, JIRA_CLIENT_SECRET
from server.auth.saas_user_auth import SaasUserAuth
from server.auth.token_manager import TokenManager
from server.constants import WEB_HOST
from storage.redis import create_redis_client
from openhands.core.logger import openhands_logger as logger
@@ -24,7 +24,7 @@ JIRA_WEBHOOKS_ENABLED = os.environ.get('JIRA_WEBHOOKS_ENABLED', '0') in (
'1',
'true',
)
JIRA_REDIRECT_URI = f'https://{WEB_HOST}/integration/jira/callback'
JIRA_REDIRECT_URI = f'{HOST_URL}/integration/jira/callback'
JIRA_SCOPES = 'read:me read:jira-user read:jira-work'
JIRA_AUTH_URL = 'https://auth.atlassian.com/authorize'
JIRA_TOKEN_URL = 'https://auth.atlassian.com/oauth/token'

View File

@@ -251,7 +251,11 @@ class TestParseWebhook:
self, jira_manager, sample_comment_webhook_payload
):
"""Test parsing comment creation webhook."""
job_context = jira_manager.parse_webhook(sample_comment_webhook_payload)
message = Message(
source=SourceType.JIRA,
message={'payload': sample_comment_webhook_payload},
)
job_context = jira_manager.parse_webhook(message)
assert job_context is not None
assert job_context.issue_id == '12345'
@@ -263,7 +267,7 @@ class TestParseWebhook:
assert job_context.base_api_url == 'https://test.atlassian.net'
def test_parse_webhook_comment_without_mention(self, jira_manager):
"""Test parsing comment without @openhands mention."""
"""Test parsing comment without @openhands mention raises error."""
payload = {
'webhookEvent': 'comment_created',
'comment': {
@@ -271,6 +275,7 @@ class TestParseWebhook:
'author': {
'emailAddress': 'user@company.com',
'displayName': 'Test User',
'accountId': 'user456',
'self': 'https://jira.company.com/rest/api/2/user?username=testuser',
},
},
@@ -281,14 +286,19 @@ class TestParseWebhook:
},
}
job_context = jira_manager.parse_webhook(payload)
assert job_context is None
message = Message(source=SourceType.JIRA, message={'payload': payload})
with pytest.raises(ValueError, match='Unrecognized jira event'):
jira_manager.parse_webhook(message)
def test_parse_webhook_issue_update_with_openhands_label(
self, jira_manager, sample_issue_update_webhook_payload
):
"""Test parsing issue update with openhands label."""
job_context = jira_manager.parse_webhook(sample_issue_update_webhook_payload)
message = Message(
source=SourceType.JIRA,
message={'payload': sample_issue_update_webhook_payload},
)
job_context = jira_manager.parse_webhook(message)
assert job_context is not None
assert job_context.issue_id == '12345'
@@ -298,7 +308,7 @@ class TestParseWebhook:
assert job_context.display_name == 'Test User'
def test_parse_webhook_issue_update_without_openhands_label(self, jira_manager):
"""Test parsing issue update without openhands label."""
"""Test parsing issue update without openhands label raises error."""
payload = {
'webhookEvent': 'jira:issue_updated',
'changelog': {'items': [{'field': 'labels', 'toString': 'bug,urgent'}]},
@@ -310,12 +320,14 @@ class TestParseWebhook:
'user': {
'emailAddress': 'user@company.com',
'displayName': 'Test User',
'accountId': 'user456',
'self': 'https://jira.company.com/rest/api/2/user?username=testuser',
},
}
job_context = jira_manager.parse_webhook(payload)
assert job_context is None
message = Message(source=SourceType.JIRA, message={'payload': payload})
with pytest.raises(ValueError, match='Unrecognized jira event'):
jira_manager.parse_webhook(message)
def test_parse_webhook_unsupported_event(self, jira_manager):
"""Test parsing webhook with unsupported event."""
@@ -324,8 +336,9 @@ class TestParseWebhook:
'issue': {'id': '12345', 'key': 'PROJ-123'},
}
job_context = jira_manager.parse_webhook(payload)
assert job_context is None
message = Message(source=SourceType.JIRA, message={'payload': payload})
with pytest.raises(ValueError, match='Unrecognized jira event'):
jira_manager.parse_webhook(message)
def test_parse_webhook_missing_required_fields(self, jira_manager):
"""Test parsing webhook with missing required fields."""
@@ -346,7 +359,8 @@ class TestParseWebhook:
},
}
job_context = jira_manager.parse_webhook(payload)
message = Message(source=SourceType.JIRA, message={'payload': payload})
job_context = jira_manager.parse_webhook(message)
assert job_context is None
@@ -373,12 +387,15 @@ class TestReceiveMessage:
jira_manager.get_issue_details = AsyncMock(
return_value=('Test Title', 'Test Description')
)
jira_manager.is_job_requested = AsyncMock(return_value=True)
jira_manager.is_repository_specified = AsyncMock(return_value=True)
jira_manager.start_job = AsyncMock()
with patch(
'integrations.jira.jira_manager.JiraFactory.create_jira_view_from_payload'
) as mock_factory:
) as mock_factory, patch(
'integrations.jira.jira_manager.JiraFactory.is_ticket_comment',
return_value=True,
):
mock_view = MagicMock(spec=JiraViewInterface)
mock_factory.return_value = mock_view
@@ -536,14 +553,14 @@ class TestReceiveMessage:
jira_manager._send_error_comment.assert_called_once()
class TestIsJobRequested:
"""Test job request validation."""
class TestIsRepositorySpecified:
"""Test repository specification validation."""
@pytest.mark.asyncio
async def test_is_job_requested_new_conversation_with_repo_match(
async def test_is_repository_specified_new_conversation_with_repo_match(
self, jira_manager, sample_job_context, sample_user_auth
):
"""Test job request validation for new conversation with repository match."""
"""Test repository validation for new conversation with repository match."""
mock_view = MagicMock(spec=JiraNewConversationView)
mock_view.saas_user_auth = sample_user_auth
mock_view.job_context = sample_job_context
@@ -565,16 +582,16 @@ class TestIsJobRequested:
mock_filter.return_value = (True, mock_repos)
message = Message(source=SourceType.JIRA, message={})
result = await jira_manager.is_job_requested(message, mock_view)
result = await jira_manager.is_repository_specified(message, mock_view)
assert result is True
assert mock_view.selected_repo == 'company/repo'
@pytest.mark.asyncio
async def test_is_job_requested_new_conversation_no_repo_match(
async def test_is_repository_specified_new_conversation_no_repo_match(
self, jira_manager, sample_job_context, sample_user_auth
):
"""Test job request validation for new conversation without repository match."""
"""Test repository validation for new conversation without repository match."""
mock_view = MagicMock(spec=JiraNewConversationView)
mock_view.saas_user_auth = sample_user_auth
mock_view.job_context = sample_job_context
@@ -597,14 +614,16 @@ class TestIsJobRequested:
mock_filter.return_value = (False, [])
message = Message(source=SourceType.JIRA, message={})
result = await jira_manager.is_job_requested(message, mock_view)
result = await jira_manager.is_repository_specified(message, mock_view)
assert result is False
jira_manager._send_repo_selection_comment.assert_called_once_with(mock_view)
@pytest.mark.asyncio
async def test_is_job_requested_exception(self, jira_manager, sample_user_auth):
"""Test job request validation when an exception occurs."""
async def test_is_repository_specified_exception(
self, jira_manager, sample_user_auth
):
"""Test repository validation when an exception occurs."""
mock_view = MagicMock(spec=JiraNewConversationView)
mock_view.saas_user_auth = sample_user_auth
jira_manager._get_repositories = AsyncMock(
@@ -612,7 +631,7 @@ class TestIsJobRequested:
)
message = Message(source=SourceType.JIRA, message={})
result = await jira_manager.is_job_requested(message, mock_view)
result = await jira_manager.is_repository_specified(message, mock_view)
assert result is False

View File

@@ -10,6 +10,7 @@ from integrations.jira.jira_view import (
JiraFactory,
JiraNewConversationView,
)
from integrations.models import Message, SourceType
class TestJiraNewConversationView:
@@ -190,3 +191,359 @@ class TestJiraViewEdgeCases:
assert new_conversation_view.job_context.issue_key == 'TEST-123'
assert new_conversation_view.selected_repo == 'test/repo1'
assert new_conversation_view.conversation_id == 'conv-123'
class TestJiraFactoryIsLabeledTicket:
"""Parameterized tests for JiraFactory.is_labeled_ticket method."""
@pytest.mark.parametrize(
'payload,expected',
[
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [{'field': 'labels', 'toString': 'openhands'}]
},
},
True,
id='issue_updated_with_openhands_label',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [
{'field': 'labels', 'toString': 'bug'},
{'field': 'labels', 'toString': 'openhands'},
]
},
},
True,
id='issue_updated_with_multiple_labels_including_openhands',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [{'field': 'labels', 'toString': 'bug,urgent'}]
},
},
False,
id='issue_updated_without_openhands_label',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {'items': []},
},
False,
id='issue_updated_with_empty_changelog_items',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {},
},
False,
id='issue_updated_with_empty_changelog',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
},
False,
id='issue_updated_without_changelog',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'changelog': {
'items': [{'field': 'labels', 'toString': 'openhands'}]
},
},
False,
id='comment_created_event_with_label',
),
pytest.param(
{
'webhookEvent': 'issue_deleted',
'changelog': {
'items': [{'field': 'labels', 'toString': 'openhands'}]
},
},
False,
id='unsupported_event_type',
),
pytest.param(
{},
False,
id='empty_payload',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [{'field': 'status', 'toString': 'In Progress'}]
},
},
False,
id='issue_updated_with_non_label_field',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [{'field': 'labels', 'fromString': 'openhands'}]
},
},
False,
id='issue_updated_with_fromString_instead_of_toString',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [
{'field': 'labels', 'toString': 'not-openhands'},
{'field': 'priority', 'toString': 'High'},
]
},
},
False,
id='issue_updated_with_mixed_fields_no_openhands',
),
],
)
def test_is_labeled_ticket(self, payload, expected):
"""Test is_labeled_ticket with various payloads."""
with patch('integrations.jira.jira_view.OH_LABEL', 'openhands'):
message = Message(source=SourceType.JIRA, message={'payload': payload})
result = JiraFactory.is_labeled_ticket(message)
assert result == expected
@pytest.mark.parametrize(
'payload,expected',
[
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [{'field': 'labels', 'toString': 'openhands-exp'}]
},
},
True,
id='issue_updated_with_staging_label',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'changelog': {
'items': [{'field': 'labels', 'toString': 'openhands'}]
},
},
False,
id='issue_updated_with_prod_label_in_staging_env',
),
],
)
def test_is_labeled_ticket_staging_labels(self, payload, expected):
"""Test is_labeled_ticket with staging environment labels."""
with patch('integrations.jira.jira_view.OH_LABEL', 'openhands-exp'):
message = Message(source=SourceType.JIRA, message={'payload': payload})
result = JiraFactory.is_labeled_ticket(message)
assert result == expected
class TestJiraFactoryIsTicketComment:
"""Parameterized tests for JiraFactory.is_ticket_comment method."""
@pytest.mark.parametrize(
'payload,expected',
[
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Please fix this @openhands'},
},
True,
id='comment_with_openhands_mention',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': '@openhands please help'},
},
True,
id='comment_starting_with_openhands_mention',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Hello @openhands!'},
},
True,
id='comment_with_openhands_mention_and_punctuation',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': '(@openhands)'},
},
True,
id='comment_with_openhands_in_parentheses',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Hey @OpenHands can you help?'},
},
True,
id='comment_with_case_insensitive_mention',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Hey @OPENHANDS!'},
},
True,
id='comment_with_uppercase_mention',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Regular comment without mention'},
},
False,
id='comment_without_mention',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Hello @openhands-agent!'},
},
False,
id='comment_with_openhands_as_prefix',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'user@openhands.com'},
},
False,
id='comment_with_openhands_in_email',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': ''},
},
False,
id='comment_with_empty_body',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {},
},
False,
id='comment_without_body',
),
pytest.param(
{
'webhookEvent': 'comment_created',
},
False,
id='comment_created_without_comment_data',
),
pytest.param(
{
'webhookEvent': 'jira:issue_updated',
'comment': {'body': 'Please fix this @openhands'},
},
False,
id='issue_updated_event_with_mention',
),
pytest.param(
{
'webhookEvent': 'issue_deleted',
'comment': {'body': '@openhands'},
},
False,
id='unsupported_event_type_with_mention',
),
pytest.param(
{},
False,
id='empty_payload',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Multiple @openhands @openhands mentions'},
},
True,
id='comment_with_multiple_mentions',
),
],
)
def test_is_ticket_comment(self, payload, expected):
"""Test is_ticket_comment with various payloads."""
with patch('integrations.jira.jira_view.INLINE_OH_LABEL', '@openhands'), patch(
'integrations.jira.jira_view.has_exact_mention'
) as mock_has_exact_mention:
from integrations.utils import has_exact_mention
mock_has_exact_mention.side_effect = (
lambda text, mention: has_exact_mention(text, mention)
)
message = Message(source=SourceType.JIRA, message={'payload': payload})
result = JiraFactory.is_ticket_comment(message)
assert result == expected
@pytest.mark.parametrize(
'payload,expected',
[
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Please fix this @openhands-exp'},
},
True,
id='comment_with_staging_mention',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': '@openhands-exp please help'},
},
True,
id='comment_starting_with_staging_mention',
),
pytest.param(
{
'webhookEvent': 'comment_created',
'comment': {'body': 'Please fix this @openhands'},
},
False,
id='comment_with_prod_mention_in_staging_env',
),
],
)
def test_is_ticket_comment_staging_labels(self, payload, expected):
"""Test is_ticket_comment with staging environment labels."""
with patch(
'integrations.jira.jira_view.INLINE_OH_LABEL', '@openhands-exp'
), patch(
'integrations.jira.jira_view.has_exact_mention'
) as mock_has_exact_mention:
from integrations.utils import has_exact_mention
mock_has_exact_mention.side_effect = (
lambda text, mention: has_exact_mention(text, mention)
)
message = Message(source=SourceType.JIRA, message={'payload': payload})
result = JiraFactory.is_ticket_comment(message)
assert result == expected