fix(jira): preserve linked description and comment text (#28584)

Preserve safe ADF hyperlinks and rich-text marks through the existing Markdown renderer, with escaping and punctuation regressions.

Refs #13604.

Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
This commit is contained in:
Neil
2026-10-14 02:57:59 -07:00
committed by GitHub
co-authored by Brennan Benson
parent b3938e6f37
commit 04e8cd99ac
6 changed files with 708 additions and 2 deletions
+3
View File
@@ -20,6 +20,9 @@
"../src/shared/**/*",
"../src/packages/process-host/src/__fixtures__/**/*",
"../src/main/gitlab/mappers.ts",
"../src/main/jira/adf-markdown.ts",
"../src/main/jira/adf-text-marks.ts",
"../src/main/jira/adf-media-destination.ts",
"../src/main/ipc/deferred-emoji-shortcode-dataset.ts",
"../src/main/ipc/worktree-branch-name.ts",
"../src/main/ipc/worktree-logic.ts",
+242
View File
@@ -0,0 +1,242 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { JiraClientForSite } from './authenticated-request'
const {
clearTokenMock,
getClientsMock,
isAuthErrorMock,
jiraRequestMock,
jiraRequestBinaryMock,
acquireMock,
releaseMock
} = vi.hoisted(() => ({
clearTokenMock: vi.fn(),
getClientsMock: vi.fn(),
isAuthErrorMock: vi.fn(),
jiraRequestMock: vi.fn(),
jiraRequestBinaryMock: vi.fn(),
acquireMock: vi.fn().mockResolvedValue(undefined),
releaseMock: vi.fn()
}))
vi.mock('./request-queue', () => ({ acquire: acquireMock, release: releaseMock }))
vi.mock('./authenticated-request', () => ({
apiBasePath: (site: { authType?: string }) =>
site.authType === 'server' ? '/rest/api/2' : '/rest/api/3',
clearToken: (...args: unknown[]) => clearTokenMock(...args),
getClients: (...args: unknown[]) => getClientsMock(...args),
isAuthError: (...args: unknown[]) => isAuthErrorMock(...args),
jiraRequest: (...args: unknown[]) => jiraRequestMock(...args),
jiraRequestBinary: (...args: unknown[]) => jiraRequestBinaryMock(...args),
JiraApiError: class JiraApiError extends Error {
status: number | null
constructor(message: string, status: number | null = null) {
super(message)
this.status = status
}
}
}))
vi.mock('./client', () => ({
clearToken: (...args: unknown[]) => clearTokenMock(...args),
getClients: (...args: unknown[]) => getClientsMock(...args),
isAuthError: (...args: unknown[]) => isAuthErrorMock(...args)
}))
function makeEntry(id = 'site-1'): JiraClientForSite {
return {
site: {
id,
siteUrl: 'https://example.atlassian.net',
email: 'ada@example.com',
displayName: 'Example Jira',
accountId: 'account-1'
},
authorization: 'Basic token'
}
}
function makeServerEntry(id = 'server-1'): JiraClientForSite {
return {
site: {
id,
siteUrl: 'https://jira.example.com',
email: '',
displayName: 'Self-hosted Jira',
accountId: 'wquintal',
authType: 'server'
},
authorization: 'Bearer pat-token'
}
}
describe('Jira ADF link mark mapping', () => {
beforeEach(async () => {
vi.clearAllMocks()
isAuthErrorMock.mockReturnValue(false)
getClientsMock.mockReturnValue([makeEntry()])
acquireMock.mockResolvedValue(undefined)
releaseMock.mockImplementation(() => {})
jiraRequestBinaryMock.mockReset()
jiraRequestMock.mockReset()
const { _resetAttachmentImageCache } = await import('./attachment-image-cache')
_resetAttachmentImageCache()
})
it('maps ADF link marks in descriptions to Markdown hyperlinks', async () => {
const { mapJiraIssue } = await import('./issues')
const issue = mapJiraIssue(makeEntry().site, {
id: 'issue-link-1',
key: 'PM-1',
fields: {
summary: 'Linked description',
description: {
type: 'doc',
version: 1,
content: [
{
type: 'paragraph',
content: [
{ type: 'text', text: 'See ' },
{
type: 'text',
text: 'Design spec',
marks: [{ type: 'link', attrs: { href: 'https://example.com/spec' } }]
},
{ type: 'text', text: '.' }
]
}
]
},
project: { id: '10000', key: 'PM', name: 'Project Management' },
issuetype: { id: '10001', name: 'Task' },
status: {
id: '1',
name: 'To Do',
statusCategory: { key: 'new', name: 'To Do' }
},
labels: [],
created: '2026-06-18T00:00:00.000Z',
updated: '2026-06-18T00:00:00.000Z'
}
})
expect(issue.description).toBe('See [Design spec](https://example.com/spec).')
})
it('leaves Server/DC plain-text descriptions unchanged', async () => {
const { mapJiraIssue } = await import('./issues')
const issue = mapJiraIssue(makeServerEntry().site, {
id: 'issue-server-1',
key: 'SRV-1',
fields: {
summary: 'Server body',
description: 'Plain body with https://example.com raw URL',
project: { id: '1', key: 'SRV', name: 'Server' },
issuetype: { id: '1', name: 'Bug' },
status: {
id: '1',
name: 'Open',
statusCategory: { key: 'new', name: 'To Do' }
},
labels: [],
created: '2026-06-18T00:00:00.000Z',
updated: '2026-06-18T00:00:00.000Z'
}
})
expect(issue.description).toBe('Plain body with https://example.com raw URL')
})
it('maps ADF link marks in comments to Markdown hyperlinks', async () => {
jiraRequestMock.mockResolvedValueOnce({
comments: [
{
id: 'comment-link-1',
body: {
type: 'doc',
version: 1,
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'Follow-up',
marks: [{ type: 'link', attrs: { href: 'https://example.com/follow-up' } }]
}
]
}
]
},
created: '2026-05-30T12:00:00.000Z',
author: { accountId: 'user-1', displayName: 'Ada' }
}
]
})
const { getIssueComments } = await import('./issues')
const comments = await getIssueComments('ALP-1', 'site-1')
expect(jiraRequestMock).toHaveBeenCalledTimes(1)
expect(jiraRequestMock).toHaveBeenCalledWith(
expect.objectContaining({ site: expect.objectContaining({ id: 'site-1' }) }),
expect.stringContaining('/issue/ALP-1/comment?')
)
expect(comments[0]?.body).toBe('[Follow-up](https://example.com/follow-up)')
})
it('leaves Server/DC plain-text comments unchanged', async () => {
getClientsMock.mockReturnValue([makeServerEntry()])
jiraRequestMock.mockResolvedValueOnce({
comments: [{ id: '1', body: '[Design spec|https://example.com/spec]', created: '2026-05-30' }]
})
const { getIssueComments } = await import('./issues')
const comments = await getIssueComments('SRV-1', 'server-1')
expect(comments[0]?.body).toBe('[Design spec|https://example.com/spec]')
expect(jiraRequestMock).toHaveBeenCalledWith(
expect.anything(),
expect.stringContaining('/rest/api/2/issue/SRV-1/comment?')
)
})
it('keeps media markdown when links and images share a description', async () => {
const resolveMedia = () => '![shot.png](data:image/png;base64,abc)'
const { adfToMarkdownText } = await import('./adf-markdown')
const markdown = adfToMarkdownText(
{
type: 'doc',
version: 1,
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'Design spec',
marks: [{ type: 'link', attrs: { href: 'https://example.com/spec' } }]
}
]
},
{
type: 'mediaSingle',
content: [
{
type: 'media',
attrs: { id: 'media-1', type: 'file', alt: 'shot.png' }
}
]
}
]
},
{ resolveMedia }
)
expect(markdown).toBe(
'[Design spec](https://example.com/spec)\n\n![shot.png](data:image/png;base64,abc)'
)
})
})
+148
View File
@@ -156,3 +156,151 @@ describe('adfToMarkdownText media', () => {
])
})
})
function paragraphWith(...content: unknown[]) {
return {
type: 'doc',
version: 1,
content: [{ type: 'paragraph', content }]
}
}
describe('adfToMarkdownText link marks', () => {
it('ignores malformed and unsupported marks without changing the source text', () => {
for (const marks of [null, {}, [null, 2], [{ type: 'textColor', attrs: { color: 'red' } }]]) {
expect(
adfToMarkdownText(paragraphWith({ type: 'text', text: 'literal [text]', marks }))
).toBe('literal [text]')
}
})
it('converts a text node link mark into a Markdown link', () => {
const markdown = adfToMarkdownText(
paragraphWith({
type: 'text',
text: 'Design spec',
marks: [{ type: 'link', attrs: { href: 'https://example.com/spec' } }]
})
)
expect(markdown).toBe('[Design spec](https://example.com/spec)')
})
it('preserves surrounding unmarked text and multiple links', () => {
const markdown = adfToMarkdownText(
paragraphWith(
{ type: 'text', text: 'See ' },
{
type: 'text',
text: 'alpha',
marks: [{ type: 'link', attrs: { href: 'https://example.com/a' } }]
},
{ type: 'text', text: ' and ' },
{
type: 'text',
text: 'beta',
marks: [{ type: 'link', attrs: { href: 'https://example.com/b' } }]
},
{ type: 'text', text: '.' }
)
)
expect(markdown).toBe('See [alpha](https://example.com/a) and [beta](https://example.com/b).')
})
it('nests strong/em/strike inside the link destination label', () => {
const markdown = adfToMarkdownText(
paragraphWith({
type: 'text',
text: 'Bold link',
marks: [
{ type: 'strong' },
{ type: 'em' },
{ type: 'strike' },
{ type: 'link', attrs: { href: 'https://example.com/x' } }
]
})
)
expect(markdown).toBe('[~~***Bold link***~~](https://example.com/x)')
})
it('wraps code marks inside the link label without nested emphasis', () => {
const markdown = adfToMarkdownText(
paragraphWith({
type: 'text',
text: 'fn()',
marks: [
{ type: 'code' },
{ type: 'strong' },
{ type: 'link', attrs: { href: 'https://example.com/api' } }
]
})
)
expect(markdown).toBe('[`fn()`](https://example.com/api)')
})
it('escapes markdown-hostile characters in labels and destinations', () => {
const href = 'https://cdn.example/x?a=1)![z](https://evil.example/y'
const markdown = adfToMarkdownText(
paragraphWith({
type: 'text',
text: 'Label] with [brackets',
marks: [{ type: 'link', attrs: { href } }]
})
)
expect(markdown).toContain('[Label\\] with \\[brackets]')
expect(markdown).toContain('%29')
expect(markdown).not.toContain('](https://evil')
})
it('renders unsafe schemes as non-clickable text', () => {
const markdown = adfToMarkdownText(
paragraphWith(
{
type: 'text',
text: 'run me',
marks: [{ type: 'link', attrs: { href: 'javascript:alert(1)' } }]
},
{ type: 'text', text: ' ' },
{
type: 'text',
text: 'local',
marks: [{ type: 'link', attrs: { href: 'file:///etc/passwd' } }]
},
{ type: 'text', text: ' ' },
{
type: 'text',
text: 'empty',
marks: [{ type: 'link', attrs: { href: '' } }]
},
{ type: 'text', text: ' ' },
{
type: 'text',
text: 'missing',
marks: [{ type: 'link', attrs: {} }]
}
)
)
expect(markdown).toBe('run me local empty missing')
expect(markdown).not.toContain('](')
})
it('preserves Unicode labels and punctuation around links', () => {
const markdown = adfToMarkdownText(
paragraphWith(
{ type: 'text', text: '読: ' },
{
type: 'text',
text: '仕様書',
marks: [{ type: 'link', attrs: { href: 'https://example.com/仕様' } }]
},
{ type: 'text', text: '!' }
)
)
expect(markdown).toBe('読: [仕様書](https://example.com/仕様)!')
})
it('leaves plain-string Server/DC bodies unchanged', () => {
expect(adfToMarkdownText('Plain wiki body with https://example.com raw')).toBe(
'Plain wiki body with https://example.com raw'
)
})
})
+8 -2
View File
@@ -1,4 +1,5 @@
import { escapeMarkdownLinkDestination } from './adf-media-destination'
import { applyAdfTextMarks, joinAdfInlineMarkdown } from './adf-text-marks'
type JiraAdfRecord = Record<string, unknown>
@@ -130,7 +131,12 @@ function renderInline(node: unknown, options?: AdfToMarkdownOptions): string {
return node
}
if (Array.isArray(node)) {
return node.map((child) => renderInline(child, options)).join('')
return joinAdfInlineMarkdown(
node.map((child) => ({
markdown: renderInline(child, options),
marks: asRecord(child).marks
}))
)
}
if (typeof node !== 'object') {
return ''
@@ -138,7 +144,7 @@ function renderInline(node: unknown, options?: AdfToMarkdownOptions): string {
const record = node as JiraAdfRecord
if (typeof record.text === 'string') {
return record.text
return applyAdfTextMarks(record.text, record.marks)
}
if (record.type === 'hardBreak') {
return '\n'
+113
View File
@@ -0,0 +1,113 @@
import { escapeMarkdownLinkDestination } from './adf-media-destination'
type JiraAdfRecord = Record<string, unknown>
function isRecord(value: unknown): value is JiraAdfRecord {
return value !== null && typeof value === 'object' && !Array.isArray(value)
}
function asRecord(value: unknown): JiraAdfRecord {
return isRecord(value) ? value : {}
}
function asArray(value: unknown): unknown[] {
return Array.isArray(value) ? value : []
}
function asString(value: unknown): string {
return typeof value === 'string' ? value : ''
}
/** Escape Markdown punctuation in source text before adding mark delimiters. */
export function escapeMarkdownLinkLabel(text: string): string {
return text
.replace(/&/g, '&amp;')
.replace(/\\/g, '\\\\')
.replace(/([`*_~[\]<>])/g, '\\$1')
.replace(/\r\n?|\n/g, ' ')
}
function markType(mark: unknown): string {
return asString(asRecord(mark).type)
}
function linkHrefFromMarks(marks: unknown[]): string | undefined {
for (const mark of marks) {
if (markType(mark) !== 'link') {
continue
}
const href = asString(asRecord(asRecord(mark).attrs).href).trim()
if (href) {
return href
}
}
return undefined
}
export function joinAdfInlineMarkdown(fragments: { markdown: string; marks: unknown }[]): string {
let result = ''
for (const { markdown, marks } of fragments) {
if (markdown.startsWith('[') && linkHrefFromMarks(asArray(marks)) && result.endsWith('!')) {
let slashCount = 0
for (let index = result.length - 2; index >= 0 && result[index] === '\\'; index -= 1) {
slashCount += 1
}
// A punctuation bang from the preceding text must not turn this link into an image.
if (slashCount % 2 === 0) {
result = `${result.slice(0, -1)}\\!`
}
}
result += markdown
}
return result
}
/** Keep formatting inside the link label so Markdown preserves both. */
export function applyAdfTextMarks(text: string, marksValue: unknown): string {
const marks = asArray(marksValue)
if (!text || marks.length === 0) {
return text
}
if (!marks.some((mark) => ['link', 'code', 'strong', 'em', 'strike'].includes(markType(mark)))) {
return text
}
const href = linkHrefFromMarks(marks)
const linkedText = href ? text.replace(/\r\n?|\n/g, ' ') : text
let formatted = linkedText
// Why: code spans cannot nest other Markdown; apply code before bold/em/strike.
if (marks.some((mark) => markType(mark) === 'code')) {
let longestBacktickRun = 0
for (const match of linkedText.matchAll(/`+/g)) {
longestBacktickRun = Math.max(longestBacktickRun, match[0].length)
}
const fence = '`'.repeat(longestBacktickRun + 1)
// Padding keeps edge backticks from joining the code-span delimiter.
const padding = /^`|`$|^ .* $/s.test(linkedText) && /[^ ]/.test(linkedText) ? ' ' : ''
formatted = `${fence}${padding}${linkedText}${padding}${fence}`
} else {
formatted = escapeMarkdownLinkLabel(formatted)
if (marks.some((mark) => markType(mark) === 'strong')) {
formatted = `**${formatted}**`
}
if (marks.some((mark) => markType(mark) === 'em')) {
formatted = `*${formatted}*`
}
if (marks.some((mark) => markType(mark) === 'strike')) {
formatted = `~~${formatted}~~`
}
}
if (!href) {
return formatted
}
const safeUrl = escapeMarkdownLinkDestination(href)
// Unsafe destinations retain the label without becoming clickable.
if (!safeUrl) {
return formatted
}
return `[${formatted}](${safeUrl})`
}
@@ -0,0 +1,194 @@
import { renderToStaticMarkup } from 'react-dom/server'
import { describe, expect, it } from 'vitest'
import { adfToMarkdownText } from '../../../../main/jira/adf-markdown'
import CommentMarkdown from './CommentMarkdown'
function renderAdf(adf: unknown): string {
return renderToStaticMarkup(
<CommentMarkdown content={adfToMarkdownText(adf)} variant="document" />
)
}
describe('CommentMarkdown Jira ADF links', () => {
it('keeps punctuation immediately before a link from turning it into an image', () => {
const markup = renderAdf({
type: 'paragraph',
content: [
{ type: 'text', text: 'Important!' },
{
type: 'text',
text: 'Design spec',
marks: [{ type: 'link', attrs: { href: 'https://example.com/spec' } }]
}
]
})
expect(markup).toContain('href="https://example.com/spec"')
expect(markup).not.toContain('<img')
})
it.each(['`edge`', ' padded ', '[bracket] <b>literal</b>', 'cost &amp; unit'])(
'preserves the literal linked label %s',
(text) => {
const content = adfToMarkdownText({
type: 'paragraph',
content: [
{
type: 'text',
text,
marks: [{ type: 'link', attrs: { href: 'https://example.com/spec' } }]
}
]
})
const markup = renderToStaticMarkup(<CommentMarkdown content={content} variant="document" />)
expect(markup).toContain('href="https://example.com/spec"')
expect(markup).not.toContain('<b>literal</b>')
expect(markup).toContain(
text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
)
}
)
it('preserves code labels with backticks at either edge', () => {
const markup = renderAdf({
type: 'paragraph',
content: [
{
type: 'text',
text: '`edge`',
marks: [{ type: 'code' }, { type: 'link', attrs: { href: 'https://example.com/code' } }]
}
]
})
expect(markup).toContain('>`edge`</code></a>')
})
it('does not allow a link label to introduce HTML or a second link', () => {
const markup = renderAdf({
type: 'paragraph',
content: [
{
type: 'text',
text: '<a href="https://evil.example">x</a> ](https://evil.example)',
marks: [{ type: 'link', attrs: { href: 'https://example.com/spec' } }]
}
]
})
expect(markup.match(/<a /g)).toHaveLength(1)
expect(markup).not.toContain('href="https://evil.example"')
})
it('keeps marked labels intact when rendered as HTML', () => {
const markup = renderAdf({
type: 'doc',
version: 1,
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'cost ` per unit',
marks: [{ type: 'link', attrs: { href: 'https://example.com/backtick' } }]
},
{ type: 'text', text: ' see `here`' }
]
},
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'a*b',
marks: [
{ type: 'strong' },
{ type: 'link', attrs: { href: 'https://example.com/star' } }
]
},
{ type: 'text', text: ' ]' }
]
},
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'bracket [label] ]',
marks: [{ type: 'link', attrs: { href: 'https://example.com/brackets' } }]
}
]
},
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'line\n\nbreak ]',
marks: [{ type: 'link', attrs: { href: 'https://example.com/newline' } }]
}
]
},
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'cost ` per unit',
marks: [
{ type: 'code' },
{ type: 'link', attrs: { href: 'https://example.com/code' } }
]
}
]
},
{
type: 'bulletList',
content: [
{
type: 'listItem',
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'list link',
marks: [{ type: 'link', attrs: { href: 'https://example.com/list' } }]
}
]
}
]
}
]
},
{
type: 'blockquote',
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
text: 'quote link',
marks: [{ type: 'link', attrs: { href: 'https://example.com/quote' } }]
}
]
}
]
}
]
})
expect(markup).toContain('href="https://example.com/backtick"')
expect(markup).toContain('>cost ` per unit</a>')
expect(markup).toContain('href="https://example.com/star"')
expect(markup).toContain('><strong>a*b</strong></a>')
expect(markup).toContain('href="https://example.com/newline"')
expect(markup).toContain('>line break ]</a>')
expect(markup).toContain('href="https://example.com/brackets"')
expect(markup).toContain('>bracket [label] ]</a>')
expect(markup).toContain('href="https://example.com/code"')
expect(markup).toContain('>cost ` per unit</code></a>')
expect(markup).toContain('href="https://example.com/list"')
expect(markup).toContain('href="https://example.com/quote"')
})
})