feat(jira): convert {info}/{note}/{warning}/{tip}/{expand} macros to Markdown (read-only)

[AnubhavSolanki]

Description

jira_to_markdown only converted the {panel} macro. The admonition macros —
{info}, {note}, {warning}, {tip} — and the {expand} collapsible block
leaked through into the output as raw {macro}...{macro} wiki markup.

This adds read-only conversion (Jira → Markdown) for those five macros,
consistent with how {panel} is handled today. Write-back (Markdown → Jira) is
intentionally out of scope per maintainer decision and can be a follow-up PR.

Fixes: #1503

Changes

• Admonitions render as a bold labelled heading that preserves the macro type,
  e.g. **ℹ️ Info: <title>** (or **ℹ️ Info** with no title), then the body.
• {expand} renders as **▸ Expand: <title>** (or **▸ Expand**). The
  {expand:Foo} shorthand and {expand:title=Foo} both yield the title;
  option-only params such as {expand:macro-id=123} render as plain
  **▸ Expand** without exposing the internal option.
• Nested (including same-type nested) macros are handled by a stack-based
  inside-out scan so they close at the correct boundary; inner code blocks are
  preserved via the existing placeholder protection.
• Plain-markdown output — no <details>, no markers, no HTML-pass protection —
  so nothing to sanitize and no write-back attack surface.

Testing

• Unit tests added/updated
• Integration tests passed
• Manual checks performed: converted {info}/{expand} (incl. nested and code-bearing) via clean_jira_text; confirmed no raw macro tags leak

TestAdmonitionAndExpandBlocks covers all four admonition types (titled/untitled),
multiline bodies, adjacent-block isolation, {expand} title/option forms,
same-type and 3-level mixed nesting (asserting no raw tags leak), inner code
preservation, and survival through the full clean_jira_text fetch path.

$ uv run pytest tests/unit/preprocessing/ -q
186 passed
$ pre-commit run --files <changed>   # ruff / ruff-format / mypy
all pass

Checklist

• Code follows project style guidelines (pre-commit clean).
• Tests added/updated for changes.
• All tests pass locally.
• Documentation updated (if needed).

[mcp-atlassian-maintainer]

Thanks for the contribution @AnubhavSolanki! The macro-specific label mapping and parameter coverage are a useful base.

1. The production read path strips the generated expand structure before users receive it, and the round-trip tests bypass that path.

See inline comments for details.

[mcp-atlassian-maintainer]

The updated production-path coverage now catches the original <details> loss, @AnubhavSolanki, and that part is fixed.

1. Multi-paragraph admonitions still break during the write-back pass.

See inline comments for details.

[mcp-atlassian-maintainer]

Thanks for the follow-up, @AnubhavSolanki. The production-path preservation and explicit boundaries now cover the multi-paragraph, list, and fenced-code cases from the last round.

1. Nested admonition and expand blocks still break during write-back.

See inline comments for details.

[mcp-atlassian-maintainer]

Thanks for fixing the nested write-back path, @AnubhavSolanki. The stack-based <details> restoration now handles both mixed nesting directions from the last round, and the focused regressions pass.

1. The read-side Jira macro parsing still leaks raw tags for same-type nesting.

See inline comments for details.

[mcp-atlassian-maintainer]

Thanks for fixing the same-type read parser, @AnubhavSolanki. The stack pass now handles the nested cases from the last round, and the focused regressions pass.

1. The visible summary is still used as the macro-type discriminator, so valid expand titles can change the Jira macro on write-back.
2. Option-only expand parameters are exposed as the visible title.

See inline comments for details.

[mcp-atlassian-maintainer]

Thanks for fixing the marker-title collision and option-only expand handling, @AnubhavSolanki. The exact-title regressions cover the last review well.

1. The new <details> protection also preserves arbitrary HTML attributes from Jira input, which bypasses the existing HTML conversion.

See inline comments for details.

[mcp-atlassian-maintainer]

Thanks for tightening the marker protection, @AnubhavSolanki. The unknown-marker and extra-attribute regressions close the HTML bypass from the last round, and the focused macro suite passes.

1. Angle-bracket text in macro titles is reinterpreted as HTML and changes during write-back.
2. An explicit expand title of Details is dropped during write-back.

See inline comments for details.

[AnubhavSolanki]

@sooperset — a human steer would help here before I keep iterating with the review bot.

State: all six review rounds are addressed, every inline thread resolved, and CI is green on Python 3.10–3.13 (lint + full test suite). The feature adds bidirectional conversion for {info}/{note}/{warning}/{tip} and {expand} macros — read (Jira→Markdown) and write-back (Markdown→Jira), with nesting support.

Two questions where a maintainer's judgment matters more than another bot round:

1. Is the bidirectional round-trip the design you want? Most of the complexity (the <details data-jira-macro> marker, nested inside-out scanning, multi-paragraph boundaries) exists purely to support converting Markdown back into the original macro on write/update. If read-only conversion (Jira→Markdown, matching how {panel} currently works) is acceptable, that would remove a large share of this diff. I'm happy either way — just want to build the version you'd actually merge.

2. Is this mergeable at the current scope? If yes, I'll finalize; if you'd like the history squashed or anything reworked, glad to.

No rush — I'll keep refining the review feedback in the meantime. Thanks for the thorough automated review; it caught several real bugs (including an HTML-attribute sanitization gap).

[mcp-atlassian-maintainer]

Thanks for the follow-up, @AnubhavSolanki. The angle-bracket and explicit Details regressions from the last round are fixed, and the focused macro suite now passes.

1. The final entity-decoding pass also rewrites user-authored raw macro parameters and can corrupt their delimiters.

See inline comments for details.

[sooperset]

Thanks @AnubhavSolanki — and I appreciate you laying the tradeoff out plainly rather than just landing the bigger version. Made this an easy call.

Let's go read-only (Jira→Markdown), same as {panel} works today.

Why:

• Consistency. {panel} is already read-only, so this keeps macro handling uniform instead of turning these five into a special case.
• The complexity and the risk both sit in write-back. Like you said, the data-jira-macro marker, the inside-out nested scanning, the HTML sanitization all exist for the reverse pass — and that's exactly where the bot found the real sanitization bug. Drop write-back and that whole attack surface goes with it.
• I'm also not sold on the round-trip in practice. Edits go through an LLM, so that invisible data-jira-macro marker has to survive the model rewriting the body, which it often won't. The read direction is a clean win with none of that risk.

If lossless write-back turns out to be something people actually hit, we can add it later as its own PR.

So for this one: strip the write-back path, keep the Jira→Markdown conversion, and squash the history when you finalize. Appreciate the thorough work on this.

[mcp-atlassian-maintainer]

Thanks for narrowing this to the read-only path, @AnubhavSolanki. Removing write-back cut the risky surface substantially, and the focused macro matrix is a solid base.

1. Preserve Markdown block boundaries around nested converted macros. The current stripping joins surrounding prose and corrupts content.

See inline comments for details.

[AnubhavSolanki]

Done — went read-only as you decided.

• Stripped the entire write-back path: no more data-jira-macro marker, inside-out <details> restoration, HTML-attribute protection, or title entity-escaping. That whole surface (and the sanitization bug with it) is gone.
• Kept the Jira→Markdown conversion. The five macros now render as bold labelled headings, matching {panel}: **ℹ️ Info: <title>**, **▸ Expand: <title>**, etc. The nested-macro handling (stack-based, same-type aware) stays since it's a read-side concern.
• Squashed the whole branch into one commit (595ae5e) rebased on latest main. Net diff is +307 across just jira.py and the test file.

PR title/description updated to reflect the read-only scope. Write-back can be a separate PR if it's ever needed.

Thanks for the clear call — agree it's the better shape.

[mcp-atlassian-maintainer]

Thanks for pushing the nested boundary regressions, @AnubhavSolanki. The exact clean_jira_text() assertions now cover the previous joining bug well.

1. HTML-looking title text is still corrupted in the production read path.
2. The read-only PR still changes fenced-code write-back without a regression.

See inline comments for details.

[mcp-atlassian-maintainer]

The exact production-path title assertions now cover the previous corruption, @AnubhavSolanki.

1. Scope the heading protection to headings generated from Jira macros. The current regex also exempts ordinary bold Jira text from HTML conversion.

See inline comments for details.

src/mcp_atlassian/preprocessing/jira.py:270 — BLOCKER: This matches any bold Jira text with one of these visible labels, not only headings generated by _convert_macro_blocks. For example, clean_jira_text('*▸ Expand: <script>alert(1)</script>*<p>body</p>') now returns the raw <script> tag. Without this extraction, the existing HTML pass removes it. Ordinary a and b tags are likewise left as raw HTML instead of converted Markdown. Required for merge: mark and protect only headings produced from supported macros, then add a regression proving a user-authored matching bold line still goes through _convert_html_to_markdown.

[mcp-atlassian-maintainer]

The exact production-path title assertions now cover the previous corruption, @AnubhavSolanki.

1. Scope the heading protection to headings generated from Jira macros. The current regex also exempts ordinary bold Jira text from HTML conversion.

See inline comments for details.

[mcp-atlassian-maintainer]

The 6 user-authored heading cases now cover the <b>, <a>, and <script> lookalikes from the last round, @AnubhavSolanki.

1. User content can still forge the fixed protection marker and bypass HTML conversion.
2. Multiline macro titles leak the private marker into the returned text.

See inline comments for details.

[mcp-atlassian-maintainer]

BLOCKER: This fixed marker isn't proof that the heading came from _convert_macro_blocks. clean_jira_text("\x00JMH\x00**▸ Expand: <script>alert(1)</script>**\x00JMH\x00") treats user-authored text as generated and returns the raw <script> tag. Required for merge: use a protection token or storage path that can't collide with the original input, then add an exact regression for forged marker input.

[mcp-atlassian-maintainer]

IMPORTANT: This extractor only matches one-line headings, while the title parser accepts newlines. clean_jira_text("{info:title=line1\nline2}<p>body</p>{info}") returns raw \x00JMH\x00 markers and escaped bold syntax. Normalize title line breaks or make the protected representation handle them, then assert that the public result contains no sentinel.

[AnubhavSolanki]

@sooperset — a scope/direction check before I iterate further, because the review bot and I are converging on diminishing returns.

Since you approved read-only, the remaining rounds have all been about one thing: protecting the generated bold heading (e.g. **ℹ️ Info: <title>**) from the HTML→Markdown pass when a title or the body contains HTML-looking text. Each fix for that has surfaced a narrower variant:

• title <summary> dropped → HTML-escape the title
• body HTML escaped the heading's ** → protect the generated heading
• protection matched user-authored bold lookalikes (XSS) → scope it with a marker
• marker is forgeable in user input + multiline titles leak it → (current open blocker)

The root issue is that the generated heading and user content share one text stream, so any in-band signal I use to tell them apart can be spoofed. I can keep hardening it, but I think it's worth your call on scope first:

Option A — merge read-only as-is (matching {panel}). {panel} already renders **title** with no HTML-escaping or protection today, so a {panel} title containing HTML has the exact same imperfect rendering. If we drop the title protection, these five macros become consistent with the existing precedent — simpler, and the whole class of edge cases disappears. Titles with literal HTML render the same way {panel} does now (imperfectly, but no security issue — the HTML pass sanitizes them).

Option B — keep hardening the protection. I fix the forgeable-marker and multiline cases (strip any pre-existing marker from input, normalize title newlines). Doable, but it makes these macros meaningfully more complex than {panel} for a corner case (HTML inside a macro title) that may be rare in practice.

My recommendation is A — match {panel}, keep the diff minimal, ship the read conversion that's the actual value here. But it's your codebase's consistency bar, so I wanted to ask rather than keep the bot loop running. Happy to do either within the hour once you point.

(State: read-only conversion works and is well-tested; CI green on 3.10–3.13; all prior threads resolved.)

[mcp-atlassian-maintainer]

The per-input UUID and DOTALL extraction close both cases from the last round, @AnubhavSolanki. The focused forged-marker and multiline-title regressions pass.

1. The storage placeholder can still collide with user content and silently duplicate generated headings.

See inline comments for details.

[mcp-atlassian-maintainer]

BLOCKER: The UUID marker itself can't collide with the source, but _extract_blocks(..., "JIRAMACRO") replaces each protected heading with the fixed \x00JIRAMACRO{index}\x00 placeholder. If the original body already contains \x00JIRAMACRO0\x00, _restore_blocks replaces both occurrences. For example, before\x00JIRAMACRO0\x00{info:title=Safe}body{info}after duplicates the Info heading and deletes the literal user text. This reintroduces the same in-band collision class the latest fix is meant to close. Required for merge: make the extraction placeholder collision-free for the current input, and add an exact clean_jira_text regression that preserves a literal \x00JIRAMACRO0\x00 next to a genuine macro.