Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
38 changes: 36 additions & 2 deletions pr_agent/tools/pr_code_suggestions.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,10 +609,44 @@ def dedent_code(self, relevant_file, relevant_lines_start, new_code_snippet):
original_initial_spaces = len(original_initial_line) - len(original_initial_line.lstrip()) # lstrip works both for spaces and tabs
suggested_initial_spaces = len(suggested_initial_line) - len(suggested_initial_line.lstrip())
delta_spaces = original_initial_spaces - suggested_initial_spaces
# Detect indentation character from original line
indent_char = '\t' if original_initial_line.startswith('\t') else ' '
Comment thread
Oxygen56 marked this conversation as resolved.
Outdated
if delta_spaces > 0:
# Detect indentation character from original line
indent_char = '\t' if original_initial_line.startswith('\t') else ' '
new_code_snippet = textwrap.indent(new_code_snippet, delta_spaces * indent_char).rstrip('\n')
elif delta_spaces < 0:
# Remove exactly -delta_spaces leading spaces from each
# non-empty line. textwrap.dedent() strips the *common*
# indent which may remove too much when lines have
# varying indentation levels (Qodo bug #3).
remove_count = -delta_spaces
dedented_lines = []
for dline in new_code_snippet.split('\n'):
if dline.strip():
leading = len(dline) - len(dline.lstrip())
dedented_lines.append(dline[min(remove_count, leading):])
else:
dedented_lines.append(dline)
new_code_snippet = '\n'.join(dedented_lines).rstrip('\n')
# Normalize all lines in the suggestion to use the original's
# indentation character. This fixes the bug where /improve
# replaces tabs with spaces in Go, Makefile, and other
# tab-indented codebases (#1858).
if indent_char == '\t':
new_lines = []
for line in new_code_snippet.split('\n'):
stripped = line.lstrip(' ')
leading_spaces = len(line) - len(stripped)
if leading_spaces > 0:
# Preserve remainder spaces when converting
# to tabs. e.g. 6 spaces → 1 tab + 2 spaces
# instead of silently dropping 2 spaces
# (Qodo bug #3).
tabs = leading_spaces // 4
remainder = leading_spaces % 4
new_lines.append('\t' * tabs + ' ' * remainder + stripped)
else:
new_lines.append(line)
new_code_snippet = '\n'.join(new_lines)
except Exception as e:
get_logger().error(f"Error when dedenting code snippet for file {relevant_file}, error: {e}")

Expand Down
60 changes: 59 additions & 1 deletion pr_agent/tools/ticket_pr_compliance_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,38 @@ def find_jira_tickets(text):
return list(tickets)


# Compiled Asana task patterns
_ASANA_TASK_URL_PATTERN = re.compile(
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
r'https://app\.asana\.com/0/(\d+)/(\d+)'
)
_ASANA_TASK_SHORT_PATTERN = re.compile(
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
r'(?:^|[^A-Za-z0-9])(?:ASANA|asana)[- ]?(\d{12,20})',
Comment thread
Oxygen56 marked this conversation as resolved.
Outdated
re.IGNORECASE,
)


def find_asana_tickets(text: str) -> list:
"""Extract Asana task references from text.

Supports both full Asana URLs and shorthand ``ASANA-123456789012``
format. Returns a list of unique task URLs.

Args:
text: The text to scan for Asana task references.

Returns:
A list of Asana task URLs.
"""
tickets = set()
for match in _ASANA_TASK_URL_PATTERN.finditer(text):
tickets.add(match.group(0))
for match in _ASANA_TASK_SHORT_PATTERN.finditer(text):
task_id = match.group(1)
if task_id:
tickets.add(f"https://app.asana.com/0/0/{task_id}")
return sorted(tickets)


def extract_ticket_links_from_pr_description(pr_description, repo_path, base_url_html='https://github.com'):
"""
Extract all ticket links from PR description
Expand Down Expand Up @@ -123,16 +155,42 @@ async def extract_tickets(git_provider):
if link not in seen:
seen.add(link)
merged.append(link)

# Also detect Asana ticket references in the PR description
asana_tickets = find_asana_tickets(user_description)
for link in asana_tickets:
if link not in seen:
seen.add(link)
merged.append(link)
asana_links = [t for t in merged if t.startswith("https://app.asana.com/")]
github_like = [t for t in merged if not t.startswith("https://app.asana.com/")]

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.

Action required

1. Asana detection github-only 🐞 Bug ≡ Correctness

find_asana_tickets() is only invoked inside the isinstance(git_provider, GithubProvider) branch
of extract_tickets(), so non-GitHub providers will never include Asana references in
related_tickets. This makes Asana ticket compliance visibility silently unavailable for other
configured providers.
Agent Prompt
## Issue description
Asana ticket detection is currently executed only when `extract_tickets()` is running in the `GithubProvider` code path. For other supported git providers, `extract_tickets()` never runs the Asana scan, so Asana references in PR descriptions are not included in `related_tickets`.

## Issue Context
- The repo supports multiple git providers.
- Docs state Asana detection “works out of the box”, but the current implementation only activates for GitHub.

## Fix Focus Areas
- pr_agent/tools/ticket_pr_compliance_check.py[134-188]

### Implementation direction
- Move Asana reference extraction (`find_asana_tickets(...)`) to a provider-agnostic location in `extract_tickets()`.
  - Use the provider base API (`git_provider.get_user_description()` exists on `GitProvider`) to obtain the text.
- For providers that are not GitHub/Azure, still return Asana placeholder ticket entries when Asana URLs are present (instead of returning `None` / no tickets).
- Keep GitHub issue fetching behavior unchanged; just merge in the Asana placeholder entries regardless of provider type.
- Add/adjust a unit test to cover a non-GitHub provider scenario (can be a minimal stub implementing `get_user_description()` and `get_pr_branch()`) to ensure Asana references are returned.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

if len(merged) > 3:
get_logger().info(f"Too many tickets (description + branch): {len(merged)}")
tickets = merged[:3]
# Reserve at least one slot for an Asana reference when
# present so it is not systematically dropped.
asana_slot = asana_links[:1]
gh_slots = 3 - len(asana_slot)
tickets = github_like[:gh_slots] + asana_slot
else:
tickets = merged
tickets_content = []

if tickets:

for ticket in tickets:
# Skip Asana URLs — these are external references,
# included for visibility but cannot be fetched via GitHub API.
if ticket.startswith("https://app.asana.com/"):
tickets_content.append({
"ticket_id": ticket,
"ticket_url": ticket,
"title": f"Asana Task: {ticket}",
"body": ("Asana task referenced in PR description. "
"Fetch task details from Asana for full context."),
"labels": "",
})
continue

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.

Action required

1. extract_tickets skips asana fetching 📎 Requirement gap ≡ Correctness

The new Asana handling explicitly avoids fetching any Asana task data and only adds a placeholder
entry, so Asana cannot be used as a real ticket provider end-to-end. This violates the requirement
to integrate an Asana Ticket Provider into the existing ticket fetching/compliance infrastructure.
Agent Prompt
## Issue description
The current Asana support only detects Asana URLs and explicitly skips fetching task details, meaning Asana is not implemented as an integrated ticket provider.

## Issue Context
PR Compliance requires Asana to be supported as a ticket provider in the same infrastructure used for other providers, so compliance checks can consume fetched Asana ticket/task data end-to-end.

## Fix Focus Areas
- pr_agent/tools/ticket_pr_compliance_check.py[150-183]
- docs/docs/core-abilities/fetching_ticket_context.md[97-112]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


repo_name, original_issue_number = git_provider._parse_issue_url(ticket)

try:
Expand Down
96 changes: 96 additions & 0 deletions tests/unittest/test_ticket_compliance.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
"""
Unit tests for Asana ticket detection in ticket_pr_compliance_check.py.

Tests cover:
- Full Asana URL detection
- Shorthand ASANA- prefix detection
- Edge cases (mixed content, no tickets, duplicates)
"""
from pr_agent.tools.ticket_pr_compliance_check import find_asana_tickets


class TestFindAsanaTickets:
"""Tests for find_asana_tickets()."""

def test_detects_full_asana_url(self):
"""Full Asana task URLs should be detected."""
text = "See https://app.asana.com/0/123456/789012 for details"
tickets = find_asana_tickets(text)
assert "https://app.asana.com/0/123456/789012" in tickets

def test_detects_asana_shorthand(self):
"""ASANA-123456789012 shorthand format should be detected."""
text = "Task ASANA-123456789012 is complete"
tickets = find_asana_tickets(text)
assert any("123456789012" in t for t in tickets)

def test_detects_multiple_tickets(self):
"""Multiple Asana references should all be found."""
text = (
"See ASANA-111111111111 and https://app.asana.com/0/22/333333333333"
)
tickets = find_asana_tickets(text)
assert len(tickets) == 2

def test_deduplicates_identical_tickets(self):
"""Duplicate references to the same task should be deduplicated."""
text = (
"ASANA-123456789012 mentioned twice: ASANA-123456789012 again"
)
tickets = find_asana_tickets(text)
assert len(tickets) == 1

def test_returns_empty_for_no_tickets(self):
"""Text without Asana references returns an empty list."""
text = "No tickets here, just regular text"
tickets = find_asana_tickets(text)
assert tickets == []

def test_returns_empty_for_empty_string(self):
"""Empty string returns an empty list."""
tickets = find_asana_tickets("")
assert tickets == []

def test_ignores_github_urls(self):
"""GitHub issue URLs should not be mistaken for Asana tickets."""
text = "Fix https://github.com/owner/repo/issues/42"
tickets = find_asana_tickets(text)
assert tickets == []

def test_shorthand_is_case_insensitive(self):
"""Both ASANA- and asana- should be detected."""
text = "asana-999999999999 lowercase works too"
tickets = find_asana_tickets(text)
assert len(tickets) == 1
assert "999999999999" in tickets[0]

def test_shorthand_mixed_case_asana(self):
"""Mixed-case shorthand like Asana-123... should also be detected."""
text = "Asana-888888888888 mixed case"
tickets = find_asana_tickets(text)
assert len(tickets) == 1
assert "888888888888" in tickets[0]

def test_shorthand_respects_word_boundary(self):
"""Text preceding the shorthand must not be alphanumeric.
NOTASANA-123 (no delimiter before ASANA) should not match."""
text = "Check NOTASANA-123456789012 in logs"
tickets = find_asana_tickets(text)
assert tickets == []

def test_tickets_are_sorted(self):
"""Returned list should be sorted alphabetically."""
text = "ASANA-222222222222 ASANA-111111111111"
tickets = find_asana_tickets(text)
assert tickets == sorted(tickets)

def test_tickets_in_pr_description_mixed_content(self):
"""Asana tickets mixed with other content in a PR description."""
text = """## Summary
Fixes ASANA-123456789012
Related to https://app.asana.com/0/99/888888888888

Also see GitHub issue #42
"""
tickets = find_asana_tickets(text)
assert len(tickets) == 2
Loading