Skip to content
7 changes: 5 additions & 2 deletions src/api/recommendations.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,16 +351,19 @@ def generate_pr_body(

body_lines.extend(
[
f"### {description} - {severity}",
"<details>",
f"<summary><b>{severity} Severity:</b> {description}</summary>",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

Escape dynamic <summary> content before rendering.

Line 355 inserts description directly into HTML. Special characters (or </summary>) can break the collapsible block rendering.

Proposed fix
+import html
@@
-        body_lines.extend(
+        safe_severity = html.escape(severity)
+        safe_description = html.escape(description)
+        body_lines.extend(
             [
                 "<details>",
-                f"<summary><b>{severity} Severity:</b> {description}</summary>",
+                f"<summary><b>{safe_severity} Severity:</b> {safe_description}</summary>",
                 "",
             ]
         )
πŸ“ 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
f"<summary><b>{severity} Severity:</b> {description}</summary>",
import html
safe_severity = html.escape(severity)
safe_description = html.escape(description)
body_lines.extend(
[
"<details>",
f"<summary><b>{safe_severity} Severity:</b> {safe_description}</summary>",
"",
]
)
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/recommendations.py` at line 355, The HTML string interpolation
inserts unescaped dynamic content (description and possibly severity) into a
<summary> tag; escape these values before rendering to prevent malformed HTML or
injection. Update the code that constructs the summary (the f-string containing
"<summary><b>{severity} Severity:</b> {description}</summary>") to apply an
HTML-escaping function (e.g., html.escape or markupsafe.escape) to description
(and severity if variable) at the point of formatting, and add the corresponding
import at the top of the module so the rendered summary contains safe, escaped
text.

"",
]
)
if reasoning:
body_lines.extend(
[
"",
f"**Rationale:** {reasoning}",
"",
]
)
body_lines.append("</details>")
body_lines.append("")

body_lines.extend(
Expand Down
113 changes: 70 additions & 43 deletions src/presentation/github_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,66 @@
}


def _build_collapsible_violations_text(violations: list[Violation]) -> str:
"""Builds a collapsible Markdown string grouped by severity for a list of violations.

Args:
violations: A list of Violation objects to format.

Returns:
A Markdown formatted string with collapsible details blocks for each severity level.
"""
if not violations:
return ""

text = ""
severity_order = ["critical", "high", "medium", "low", "info"]
severity_groups: dict[str, list[Violation]] = {s: [] for s in severity_order}

for violation in violations:
sev = violation.severity.value if hasattr(violation.severity, "value") else str(violation.severity)
if sev in severity_groups:
severity_groups[sev].append(violation)
else:
severity_groups["low"].append(violation)

for severity in severity_order:
if severity_groups[severity]:
emoji = SEVERITY_STR_EMOJI.get(severity, "βšͺ")
count = len(severity_groups[severity])

text += "<details>\n"
text += f"<summary><b>{emoji} {severity.title()} Severity ({count})</b></summary>\n\n"

for violation in severity_groups[severity]:
text += f"### {violation.rule_description or 'Unknown Rule'}\n"
text += f"{violation.message}\n"
if violation.how_to_fix:
text += f"**How to fix:** {violation.how_to_fix}\n"
text += "\n"

text += "</details>\n\n"

return text


def format_check_run_output(
violations: list[Violation],
error: str | None = None,
repo_full_name: str | None = None,
installation_id: int | None = None,
) -> dict[str, Any]:
"""Format violations for check run output."""
"""Format violations for check run output.

Args:
violations: List of rule violations to report.
error: Optional error message if rule processing failed entirely.
repo_full_name: The full repository name (e.g., owner/repo).
installation_id: The GitHub App installation ID.

Returns:
A dictionary matching the GitHub Check Run Output schema containing title, summary, and text.
"""
if error:
# Check if it's a missing rules file error
if "rules not configured" in error.lower() or "rules file not found" in error.lower():
Expand Down Expand Up @@ -75,7 +128,7 @@ def format_check_run_output(
}

# Group violations by severity
severity_order = ["critical", "high", "medium", "low"]
severity_order = ["critical", "high", "medium", "low", "info"]
severity_groups: dict[str, list[Violation]] = {s: [] for s in severity_order}

for violation in violations:
Expand All @@ -99,22 +152,9 @@ def format_check_run_output(

# Build detailed text
text = "# Watchflow Rule Violations\n\n"

for severity in severity_order:
if severity_groups[severity]:
emoji = SEVERITY_STR_EMOJI.get(severity, "βšͺ")
text += f"## {emoji} {severity.title()} Severity\n\n"

for violation in severity_groups[severity]:
text += f"### {violation.rule_description or 'Unknown Rule'}\n\n"
text += f"{violation.message}\n\n"
text += f"Rule validation failed with severity: **{violation.severity}**\n"
if violation.how_to_fix:
text += f"**How to fix:** {violation.how_to_fix}\n"
text += "\n"

text += _build_collapsible_violations_text(violations)
text += "---\n"
text += "*To configure rules, edit the `.watchflow/rules.yaml` file in this repository.*"
text += "πŸ’‘ *To configure rules, edit the `.watchflow/rules.yaml` file in this repository.*"

return {"title": f"{len(violations)} rule violations found", "summary": summary, "text": text}

Expand Down Expand Up @@ -151,35 +191,22 @@ def format_rules_not_configured_comment(


def format_violations_comment(violations: list[Violation]) -> str:
"""Format violations as a GitHub comment."""
comment = "## 🚨 Watchflow Rule Violations Detected\n\n"

# Group violations by severity
severity_order = ["critical", "high", "medium", "low"]
severity_groups: dict[str, list[Violation]] = {s: [] for s in severity_order}
"""Format violations as a GitHub comment.

for violation in violations:
sev = violation.severity.value if hasattr(violation.severity, "value") else str(violation.severity)
if sev in severity_groups:
severity_groups[sev].append(violation)
Args:
violations: List of rule violations to include in the comment.

# Add violations by severity (most severe first)
for severity in severity_order:
if severity_groups[severity]:
emoji = SEVERITY_STR_EMOJI.get(severity, "βšͺ")
comment += f"### {emoji} {severity.title()} Severity\n\n"

for violation in severity_groups[severity]:
comment += f"### {violation.rule_description or 'Unknown Rule'}\n\n"
comment += f"{violation.message}\n\n"
comment += f"Rule validation failed with severity: **{violation.severity}**\n"
if violation.how_to_fix:
comment += f"**How to fix:** {violation.how_to_fix}\n"
comment += "\n"
Returns:
A Markdown formatted string suitable for a Pull Request timeline comment.
Returns an empty string if there are no violations.
"""
if not violations:
return ""

comment = f"### πŸ›‘οΈ Watchflow Governance Checks\n**Status:** ❌ {len(violations)} Violations Found\n\n"
comment += _build_collapsible_violations_text(violations)
comment += "---\n"
comment += "*This comment was automatically generated by [Watchflow](https://watchflow.dev).*\n"
comment += "*To configure rules, edit the `.watchflow/rules.yaml` file in this repository.*"
comment += "πŸ’‘ *Reply with `@watchflow ack [reason]` to override these rules, or `@watchflow help` for commands.*"

return comment

Expand Down Expand Up @@ -261,7 +288,7 @@ def format_acknowledgment_check_run(
{format_acknowledgment_summary(acknowledgable_violations, acknowledgments)}

**Violations Requiring Fixes:**
{format_violations_for_check_run(violations)}
{_build_collapsible_violations_text(violations)}

Please address the remaining violations or acknowledge them with a valid reason.
"""
Expand Down
35 changes: 29 additions & 6 deletions tests/unit/presentation/test_github_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,43 @@ def test_format_violations_comment_groups_by_severity():

comment = format_violations_comment(violations)

assert "## 🚨 Watchflow Rule Violations Detected" in comment
assert "### πŸ”΄ Critical Severity" in comment
assert "### 🟠 High Severity" in comment
assert "### πŸ›‘οΈ Watchflow Governance Checks" in comment
assert "**Status:** ❌ 3 Violations Found" in comment
assert "<summary><b>πŸ”΄ Critical Severity (1)</b></summary>" in comment
assert "<summary><b>🟠 High Severity (2)</b></summary>" in comment
assert "### Rule 2" in comment
assert "### Rule 1" in comment
assert "### Rule 3" in comment
assert "Fix 1" in comment
assert "Fix 2" in comment
assert "πŸ’‘ *Reply with `@watchflow ack [reason]`" in comment


def test_format_violations_comment_empty():
comment = format_violations_comment([])
assert "## 🚨 Watchflow Rule Violations Detected" in comment
assert "---" in comment
assert comment == ""


def test_build_collapsible_violations_text_fallback_severity():
"""Test that a violation with an unknown severity falls back to 'low' severity bucket."""
# Create a violation and use object.__setattr__ to bypass Pydantic validation for testing
v = Violation(rule_description="Weird Rule", severity=Severity.LOW, message="Weird error")
object.__setattr__(v, "severity", "super_critical_unknown")

comment = format_violations_comment([v])

# It should fall back to low severity
assert "<summary><b>🟒 Low Severity (1)</b></summary>" in comment
assert "Weird error" in comment


def test_build_collapsible_violations_text_info_severity():
"""Test that INFO severity violations are correctly formatted."""
v = Violation(rule_description="Info Rule", severity=Severity.INFO, message="Just an info message")
comment = format_violations_comment([v])

assert "<summary><b>βšͺ Info Severity (1)</b></summary>" in comment
assert "Just an info message" in comment


def test_format_check_run_output_success():
Expand All @@ -46,7 +69,7 @@ def test_format_check_run_output_with_violations():

assert "1 rule violations found" in output["title"]
assert "🚨 1 violations found: 1 high" in output["summary"]
assert "## 🟠 High Severity" in output["text"]
assert "<summary><b>🟠 High Severity (1)</b></summary>" in output["text"]
assert "### Missing Issue" in output["text"]


Expand Down