From 25921bf4e0c91e0f3ba74482479a1510d8f17686 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 4 Nov 2026 09:51:36 -0500 Subject: [PATCH 1/4] fix(adf): decode markdown escapes and entities in text A page read as markdown escapes punctuation and encodes entities, and the converter stored those as literal text, so each read and write back added visible backslashes and entity names. Decode them for text and image alt text, as CommonMark specifies. Code spans and blocks keep their source. --- shared/adf/convert.go | 33 ++++++++++++++++++++++-- shared/adf/convert_test.go | 40 ++++++++++++++++++++++++++++++ tools/cfl/CHANGELOG.md | 1 + tools/cfl/pkg/md/roundtrip_test.go | 23 +++++++++++++++++ tools/jtk/CHANGELOG.md | 1 + 5 files changed, 96 insertions(+), 2 deletions(-) diff --git a/shared/adf/convert.go b/shared/adf/convert.go index 5ff97d68..a6d9912b 100644 --- a/shared/adf/convert.go +++ b/shared/adf/convert.go @@ -2,6 +2,7 @@ package adf import ( + "bytes" "encoding/json" "strings" @@ -11,6 +12,7 @@ import ( "github.com/yuin/goldmark/extension" extast "github.com/yuin/goldmark/extension/ast" "github.com/yuin/goldmark/text" + "github.com/yuin/goldmark/util" ) // mdParser is a goldmark parser for standard markdown input. @@ -369,11 +371,38 @@ func (c *converter) convertInlineChildren(n ast.Node) []*Node { return nodes } +// decodeInlineText applies CommonMark's backslash escapes and entity references, +// which goldmark leaves in a text segment's raw source for its renderer to decode. +// A code span or block keeps its source as written, so it does not come through here. +func decodeInlineText(src []byte) string { + var out bytes.Buffer + for i := 0; i < len(src); i++ { + switch { + case src[i] == '\\' && i+1 < len(src) && util.IsPunct(src[i+1]): + i++ + out.WriteByte(src[i]) + case src[i] == '&': + if end := bytes.IndexByte(src[i:], ';'); end > 0 { + ref := src[i : i+end+1] + if decoded := util.ResolveNumericReferences(util.ResolveEntityNames(ref)); !bytes.Equal(decoded, ref) { + out.Write(decoded) + i += end + continue + } + } + out.WriteByte(src[i]) + default: + out.WriteByte(src[i]) + } + } + return out.String() +} + // convertInlineNode converts an inline AST node to ADF text node(s). func (c *converter) convertInlineNode(n ast.Node, marks []*Mark) []*Node { switch node := n.(type) { case *ast.Text: - txt := string(node.Segment.Value(c.source)) + txt := decodeInlineText(node.Segment.Value(c.source)) if txt == "" { return nil } @@ -450,7 +479,7 @@ func (c *converter) convertInlineNode(n ast.Node, marks []*Mark) []*Node { var altBuilder strings.Builder for child := node.FirstChild(); child != nil; child = child.NextSibling() { if textNode, ok := child.(*ast.Text); ok { - altBuilder.Write(textNode.Segment.Value(c.source)) + altBuilder.WriteString(decodeInlineText(textNode.Segment.Value(c.source))) } } alt := altBuilder.String() diff --git a/shared/adf/convert_test.go b/shared/adf/convert_test.go index 26e80e09..78bd8ebb 100644 --- a/shared/adf/convert_test.go +++ b/shared/adf/convert_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "strings" "testing" "github.com/open-cli-collective/atlassian-go/testutil" @@ -914,3 +915,42 @@ func TestToPlainText_NodesWithTextInAttrs(t *testing.T) { "https://example.com/embed\n" testutil.Equal(t, doc.ToPlainText(), want) } + +func TestToDocument_DecodesMarkdownEscapesInText(t *testing.T) { + t.Parallel() + cases := []struct{ name, markdown, want string }{ + {"escaped underscore", `:rotating\_light: x`, ":rotating_light: x"}, + {"escaped star and bracket", `\*star* \[b]`, "*star* [b]"}, + {"escaped backslash", `back\\slash`, `back\slash`}, + {"backslash before a letter stays", `back\slash`, `back\slash`}, + {"named entity", `A > B & C`, "A > B & C"}, + {"numeric entities", `> >`, "> >"}, + {"unknown entity stays", `&nosuchentity; & alone`, "&nosuchentity; & alone"}, + {"escaped entity stays literal", `\>`, ">"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + doc := ToDocument(c.markdown) + var got strings.Builder + for _, n := range doc.Content[0].Content { + got.WriteString(n.Text) + } + if got.String() != c.want { + t.Errorf("text = %q, want %q", got.String(), c.want) + } + }) + } +} + +func TestToDocument_KeepsEscapesInCode(t *testing.T) { + t.Parallel() + doc := ToDocument("`a\\_b >`") + if got := doc.Content[0].Content[0].Text; got != `a\_b >` { + t.Errorf("code span text = %q", got) + } + doc = ToDocument("```\na\\_b >\n```") + if got := doc.Content[0].Content[0].Text; got != "a\\_b >" { + t.Errorf("code block text = %q", got) + } +} diff --git a/tools/cfl/CHANGELOG.md b/tools/cfl/CHANGELOG.md index 1f57cbb9..908622e2 100644 --- a/tools/cfl/CHANGELOG.md +++ b/tools/cfl/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- `page edit` and `page create` no longer store markdown escapes and entities as literal text. Reading a page as markdown and writing it back used to add visible backslashes and `>` to its text. - `attachment list --unused` now reads ADF pages whose storage body is empty, matches attachment references structurally (storage `ri:attachment` elements and ADF media nodes) instead of by filename substring, and pages through all attachments until `--limit` unused results are found ([#456](https://github.com/open-cli-collective/atlassian-cli/issues/456)) - `page view` and `page edit` now fall back to ADF format when the Confluence API returns empty storage content for cloud editor pages ([#150](https://github.com/open-cli-collective/atlassian-cli/issues/150)) diff --git a/tools/cfl/pkg/md/roundtrip_test.go b/tools/cfl/pkg/md/roundtrip_test.go index 64a3d171..4b2b87b7 100644 --- a/tools/cfl/pkg/md/roundtrip_test.go +++ b/tools/cfl/pkg/md/roundtrip_test.go @@ -1,6 +1,7 @@ package md import ( + "encoding/json" "strings" "testing" @@ -238,3 +239,25 @@ func TestRoundtrip_NestedMacroInParagraph(t *testing.T) { testutil.True(t, infoStart < tocPos, "[INFO] should come before [TOC]") testutil.True(t, tocPos < infoEnd, "[TOC] should come before [/INFO]") } + +// TestRoundtrip_PlainTextThroughStorageRead verifies that a page read as markdown +// and written back keeps its text: the read escapes markdown punctuation and the +// write has to decode it, or every round trip adds visible backslashes. +func TestRoundtrip_PlainTextThroughStorageRead(t *testing.T) { + t.Parallel() + storage := `

:rotating_light: a_b_c *star* [b] A > B 1 & 2 <tag> back\slash

` + want := `:rotating_light: a_b_c *star* [b] A > B 1 & 2 back\slash` + + markdown, err := FromConfluenceStorage(storage) + testutil.RequireNoError(t, err) + adfJSON, err := ToADF([]byte(markdown)) + testutil.RequireNoError(t, err) + + var doc ADFDocument + testutil.RequireNoError(t, json.Unmarshal([]byte(adfJSON), &doc)) + var got strings.Builder + for _, n := range doc.Content[0].Content { + got.WriteString(n.Text) + } + testutil.Equal(t, got.String(), want) +} diff --git a/tools/jtk/CHANGELOG.md b/tools/jtk/CHANGELOG.md index 25c65a97..dab37cf4 100644 --- a/tools/jtk/CHANGELOG.md +++ b/tools/jtk/CHANGELOG.md @@ -59,6 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Markdown escapes and entities in issue and comment text now decode, so `\_` is stored as `_` and `>` as `>`. Code spans and code blocks keep them as written. - `development get` now returns every pull request linked to an issue instead of stopping at 100. Jira's dev-status detail API caps its response at 100 and ignores paging parameters, so pull requests are now paged from Jira's Graph Store issue-to-PR relationship, with dev-status kept as a fallback that reports `100+` and warns on stderr when it may be truncated. The repository column also now comes from each pull request instead of repeating the first one's repository for every row. - `config show` now reports values set in the shared `atlassian-cli/config.yml` store (url, email, default project) with their shared source, and its path row names the config file the runtime actually reads, including the prior shared location on macOS before `jtk init` relocates it. Previously these rows showed `-` and the legacy per-tool path even though jtk used the shared values. Thanks to @jurij for the fix. ([#483](https://github.com/open-cli-collective/atlassian-cli/pull/483)) - Smart links and mentions no longer disappear from `issues get` descriptions, comment output, and rich-text custom field values. Inline and block cards now render as their URL and mentions as their display text; previously they rendered as empty text. ([#495](https://github.com/open-cli-collective/atlassian-cli/pull/495)) From b3135fb43896dd3a14895ddd0611a25652a8c582 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 4 Nov 2026 09:55:56 -0500 Subject: [PATCH 2/4] fix(cfl): keep literal entities across a page read and write Scan entity references exactly, so a bare ampersand does not swallow the text after it, and escape an entity a page shows literally on the storage read, so the write's decoding does not strip it. --- shared/adf/convert.go | 35 ++++++++++++++++++++++-------- shared/adf/convert_test.go | 2 ++ tools/cfl/pkg/md/from_html.go | 24 +++++++++++++++++++- tools/cfl/pkg/md/roundtrip_test.go | 21 ++++++++++++++++++ 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/shared/adf/convert.go b/shared/adf/convert.go index a6d9912b..077fd177 100644 --- a/shared/adf/convert.go +++ b/shared/adf/convert.go @@ -377,27 +377,44 @@ func (c *converter) convertInlineChildren(n ast.Node) []*Node { func decodeInlineText(src []byte) string { var out bytes.Buffer for i := 0; i < len(src); i++ { - switch { - case src[i] == '\\' && i+1 < len(src) && util.IsPunct(src[i+1]): + if src[i] == '\\' && i+1 < len(src) && util.IsPunct(src[i+1]) { i++ out.WriteByte(src[i]) - case src[i] == '&': - if end := bytes.IndexByte(src[i:], ';'); end > 0 { - ref := src[i : i+end+1] + continue + } + if src[i] == '&' { + if end := entityEnd(src[i:]); end > 0 { + ref := src[i : i+end] if decoded := util.ResolveNumericReferences(util.ResolveEntityNames(ref)); !bytes.Equal(decoded, ref) { out.Write(decoded) - i += end + i += end - 1 continue } } - out.WriteByte(src[i]) - default: - out.WriteByte(src[i]) } + out.WriteByte(src[i]) } return out.String() } +// entityEnd returns the length of the entity reference src starts with: an ampersand, +// an optional #, alphanumerics and a semicolon. It returns 0 when src does not start +// with one, so a bare ampersand never reaches past the text that follows it. +func entityEnd(src []byte) int { + i := 1 + if i < len(src) && src[i] == '#' { + i++ + } + start := i + for i < len(src) && util.IsAlphaNumeric(src[i]) { + i++ + } + if i == start || i >= len(src) || src[i] != ';' { + return 0 + } + return i + 1 +} + // convertInlineNode converts an inline AST node to ADF text node(s). func (c *converter) convertInlineNode(n ast.Node, marks []*Mark) []*Node { switch node := n.(type) { diff --git a/shared/adf/convert_test.go b/shared/adf/convert_test.go index 78bd8ebb..a4caf4d0 100644 --- a/shared/adf/convert_test.go +++ b/shared/adf/convert_test.go @@ -927,6 +927,8 @@ func TestToDocument_DecodesMarkdownEscapesInText(t *testing.T) { {"numeric entities", `> >`, "> >"}, {"unknown entity stays", `&nosuchentity; & alone`, "&nosuchentity; & alone"}, {"escaped entity stays literal", `\>`, ">"}, + {"bare ampersand before an escape and an entity", `R&D a\_b > c`, "R&D a_b > c"}, + {"bare ampersand before an escaped entity", `a & \>`, "a & >"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/tools/cfl/pkg/md/from_html.go b/tools/cfl/pkg/md/from_html.go index ef66655a..cf66f6e4 100644 --- a/tools/cfl/pkg/md/from_html.go +++ b/tools/cfl/pkg/md/from_html.go @@ -57,10 +57,11 @@ func FromConfluenceStorageWithOptions(html string, opts ConvertOptions) (string, ), ) - markdown, err := conv.ConvertString(html) + markdown, err := conv.ConvertString(literalEntityInStorage.ReplaceAllString(html, literalAmpersand+"$1")) if err != nil { return "", err } + markdown = restoreLiteralEntities(markdown) // Replace placeholders with actual bracket syntax markdown = replaceMacroPlaceholders(markdown, macroMap) @@ -74,6 +75,27 @@ func FromConfluenceStorageWithOptions(html string, opts ConvertOptions) (string, return strings.TrimSpace(markdown), nil } +// literalEntityInStorage matches page text that shows an entity literally, such as +// &gt; for a page displaying ">". The converter renders it the same as the +// character > itself, so it is marked before conversion to tell the two apart. +var literalEntityInStorage = regexp.MustCompile(`&(#?[A-Za-z0-9]+;)`) + +// literalAmpersand is a private-use character standing in for the ampersand of a +// literal entity during conversion. +const literalAmpersand = "\uE000" + +// restoreLiteralEntities writes each marked ampersand back: escaped in text, so the +// markdown parser keeps the entity literal, and as written in code, where it already is. +func restoreLiteralEntities(markdown string) string { + if !strings.Contains(markdown, literalAmpersand) { + return markdown + } + protected, regions := protectCodeRegions([]byte(markdown)) + escaped := strings.ReplaceAll(string(protected), literalAmpersand, `\&`) + restored := restoreCodeRegions([]byte(escaped), regions) + return strings.ReplaceAll(string(restored), literalAmpersand, "&") +} + // macroPlaceholder stores the bracket syntax for a macro placeholder type macroPlaceholder struct { openTag string // e.g., "[INFO title=Title]" diff --git a/tools/cfl/pkg/md/roundtrip_test.go b/tools/cfl/pkg/md/roundtrip_test.go index 4b2b87b7..63a106e4 100644 --- a/tools/cfl/pkg/md/roundtrip_test.go +++ b/tools/cfl/pkg/md/roundtrip_test.go @@ -261,3 +261,24 @@ func TestRoundtrip_PlainTextThroughStorageRead(t *testing.T) { } testutil.Equal(t, got.String(), want) } + +// A page that shows an entity literally keeps it across a read and write, while the +// character that the same markdown text would otherwise decode to stays a character. +func TestRoundtrip_LiteralEntityThroughStorageRead(t *testing.T) { + t.Parallel() + storage := `

&gt; &amp; R&D a_b > c &gt;

` + want := `> & R&D a_b > c >` + + markdown, err := FromConfluenceStorage(storage) + testutil.RequireNoError(t, err) + adfJSON, err := ToADF([]byte(markdown)) + testutil.RequireNoError(t, err) + + var doc ADFDocument + testutil.RequireNoError(t, json.Unmarshal([]byte(adfJSON), &doc)) + var got strings.Builder + for _, n := range doc.Content[0].Content { + got.WriteString(n.Text) + } + testutil.Equal(t, got.String(), want) +} From 26b436114f2e7e3d28326c2e09397ebf1d728af5 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 4 Nov 2026 09:59:37 -0500 Subject: [PATCH 3/4] fix(cfl): mark literal entities on text nodes only Use the converter's text hook, so attributes and code stay as stored and literal backticks around an entity no longer read as code. --- tools/cfl/pkg/md/from_html.go | 41 ++++++++++++++++-------------- tools/cfl/pkg/md/roundtrip_test.go | 14 ++++++++-- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/tools/cfl/pkg/md/from_html.go b/tools/cfl/pkg/md/from_html.go index cf66f6e4..bcf460e1 100644 --- a/tools/cfl/pkg/md/from_html.go +++ b/tools/cfl/pkg/md/from_html.go @@ -1,6 +1,7 @@ package md import ( + "bytes" "fmt" "regexp" "strconv" @@ -57,11 +58,13 @@ func FromConfluenceStorageWithOptions(html string, opts ConvertOptions) (string, ), ) - markdown, err := conv.ConvertString(literalEntityInStorage.ReplaceAllString(html, literalAmpersand+"$1")) + conv.Register.TextTransformer(markLiteralEntities, converter.PriorityEarly) + conv.Register.PostRenderer(escapeLiteralEntities, converter.PriorityLate) + + markdown, err := conv.ConvertString(html) if err != nil { return "", err } - markdown = restoreLiteralEntities(markdown) // Replace placeholders with actual bracket syntax markdown = replaceMacroPlaceholders(markdown, macroMap) @@ -75,25 +78,25 @@ func FromConfluenceStorageWithOptions(html string, opts ConvertOptions) (string, return strings.TrimSpace(markdown), nil } -// literalEntityInStorage matches page text that shows an entity literally, such as -// &gt; for a page displaying ">". The converter renders it the same as the -// character > itself, so it is marked before conversion to tell the two apart. -var literalEntityInStorage = regexp.MustCompile(`&(#?[A-Za-z0-9]+;)`) +// literalEntity matches text a page shows as an entity, such as ">". The converter +// writes it the same as the character > itself, so a parser reading the markdown back +// could not tell them apart. +var literalEntity = regexp.MustCompile(`&(#?[A-Za-z0-9]+;)`) -// literalAmpersand is a private-use character standing in for the ampersand of a -// literal entity during conversion. -const literalAmpersand = "\uE000" +// literalAmpersand marks a literal entity's ampersand during conversion. It is a Unicode +// noncharacter, which page text does not hold. +const literalAmpersand = "\uFDD0" -// restoreLiteralEntities writes each marked ampersand back: escaped in text, so the -// markdown parser keeps the entity literal, and as written in code, where it already is. -func restoreLiteralEntities(markdown string) string { - if !strings.Contains(markdown, literalAmpersand) { - return markdown - } - protected, regions := protectCodeRegions([]byte(markdown)) - escaped := strings.ReplaceAll(string(protected), literalAmpersand, `\&`) - restored := restoreCodeRegions([]byte(escaped), regions) - return strings.ReplaceAll(string(restored), literalAmpersand, "&") +// markLiteralEntities runs on text nodes only, before the converter's own escaping, so +// attributes and code are left alone. +func markLiteralEntities(_ converter.Context, content string) string { + return literalEntity.ReplaceAllString(content, literalAmpersand+"$1") +} + +// escapeLiteralEntities writes each marked ampersand as a markdown escape, which the +// markdown parser keeps literal. +func escapeLiteralEntities(_ converter.Context, markdown []byte) []byte { + return bytes.ReplaceAll(markdown, []byte(literalAmpersand), []byte(`\&`)) } // macroPlaceholder stores the bracket syntax for a macro placeholder diff --git a/tools/cfl/pkg/md/roundtrip_test.go b/tools/cfl/pkg/md/roundtrip_test.go index 63a106e4..cde38ee8 100644 --- a/tools/cfl/pkg/md/roundtrip_test.go +++ b/tools/cfl/pkg/md/roundtrip_test.go @@ -266,8 +266,8 @@ func TestRoundtrip_PlainTextThroughStorageRead(t *testing.T) { // character that the same markdown text would otherwise decode to stays a character. func TestRoundtrip_LiteralEntityThroughStorageRead(t *testing.T) { t.Parallel() - storage := `

&gt; &amp; R&D a_b > c &gt;

` - want := `> & R&D a_b > c >` + storage := "

&gt; &amp; R&D a_b > c `&gt;` &gt;

" + want := "> & R&D a_b > c `>` >" markdown, err := FromConfluenceStorage(storage) testutil.RequireNoError(t, err) @@ -282,3 +282,13 @@ func TestRoundtrip_LiteralEntityThroughStorageRead(t *testing.T) { } testutil.Equal(t, got.String(), want) } + +// Only text nodes are marked: a link keeps its query string as it was stored. +func TestFromConfluenceStorage_LiteralEntityMarkerStaysOutOfLinks(t *testing.T) { + t.Parallel() + markdown, err := FromConfluenceStorage(`

l

`) + testutil.RequireNoError(t, err) + testutil.Equal(t, strings.Contains(markdown, "%EE%B7%90"), false) + testutil.Equal(t, strings.Contains(markdown, literalAmpersand), false) + testutil.Contains(t, markdown, "a=1") +} From 06c8e5e7694a071df73d3086248a3dc2dbd154c8 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 4 Nov 2026 10:01:59 -0500 Subject: [PATCH 4/4] test(adf): cover image alt text decoding --- shared/adf/convert_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/shared/adf/convert_test.go b/shared/adf/convert_test.go index a4caf4d0..785ad4e8 100644 --- a/shared/adf/convert_test.go +++ b/shared/adf/convert_test.go @@ -956,3 +956,11 @@ func TestToDocument_KeepsEscapesInCode(t *testing.T) { t.Errorf("code block text = %q", got) } } + +func TestToDocument_DecodesImageAltText(t *testing.T) { + t.Parallel() + doc := ToDocument(`![a\_b >](https://example.com/i.png)`) + if got := doc.Content[0].Content[0].Text; got != "a_b >" { + t.Errorf("alt text = %q", got) + } +}