From cdd5ceb72b29f81234b30845c15fa7014a35e37b Mon Sep 17 00:00:00 2001 From: Nemo Date: Tue, 28 Jul 2026 06:34:20 +0800 Subject: [PATCH] fix(jira): render issue description/comment images with lightbox (#8938) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(jira): render issue images and open them in a lightbox Jira ADF media nodes were dropped when converting descriptions/comments to Markdown, so screenshots never appeared in the Tasks drawer. Download image attachments with authenticated Jira API access, embed them as data URLs on issue/comment detail loads, and add a viewport-centered lightbox. Closing with X/Esc only dismisses the preview, not the issue sheet. * fix(jira): open comment images in the same lightbox as description Jira issue comments still used compact markdown, so screenshots rendered but could not expand. Use the document renderer for comment bodies, add a regression test for the expand control, and sync MarkdownImageLightbox locale keys. * fix(jira): harden inline image handling * fix(jira): harden inline image discovery, escaping, and downloads Address PR review findings: correct media-attachment pairing, Server/DC attachment lookup base path, markdown-safe external URLs, wider HTML discovery with gated alt fallback, concurrent downloads outside the API semaphore, and a main-process attachment data-URL cache with lower caps. * fix(jira): Option A multi-same-name attachments and post-map media warns Fix discovery so repeated alts (image.png) get distinct attachment ids, flush resolution warns after ADF mapping using attachment-only stats, clear attachment cache on clearToken with epoch-guarded singleflight, and add Server comment path plus release-before-binary regression tests. * fix(jira): simplify comment media request skip condition Only needingCount determines whether to skip the attachment metadata request — htmlIds alone cannot produce a download without needing media. Add type annotation for mediaAttrs for clarity. --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> --- src/main/jira/adf-markdown.test.ts | 158 ++++++++ src/main/jira/adf-markdown.ts | 155 ++++++-- src/main/jira/adf-media-destination.ts | 56 +++ src/main/jira/attachment-discovery.ts | 99 +++++ src/main/jira/attachment-image-cache.test.ts | 61 +++ src/main/jira/attachment-image-cache.ts | 169 +++++++++ src/main/jira/attachment-images.test.ts | 351 ++++++++++++++++++ src/main/jira/attachment-images.ts | 252 +++++++++++++ src/main/jira/attachment-meta.ts | 56 +++ src/main/jira/client.test.ts | 34 ++ src/main/jira/client.ts | 36 ++ src/main/jira/issues.test.ts | 259 ++++++++++++- src/main/jira/issues.ts | 259 ++++++++++++- src/main/runtime/rpc/methods/jira.test.ts | 23 ++ src/main/runtime/rpc/methods/jira.ts | 35 +- .../src/components/JiraIssueWorkspace.tsx | 3 + src/renderer/src/components/TaskPage.tsx | 4 +- .../sidebar/CommentMarkdown.test.tsx | 24 ++ .../components/sidebar/CommentMarkdown.tsx | 10 +- .../sidebar/MarkdownImageLightbox.test.tsx | 68 ++++ .../sidebar/MarkdownImageLightbox.tsx | 82 ++++ .../comment-markdown-element-renderers.tsx | 50 ++- src/renderer/src/i18n/locales/en.json | 5 + src/renderer/src/i18n/locales/es.json | 5 + src/renderer/src/i18n/locales/ja.json | 5 + src/renderer/src/i18n/locales/ko.json | 5 + src/renderer/src/i18n/locales/zh.json | 5 + .../src/runtime/runtime-jira-client.test.ts | 68 +++- .../src/runtime/runtime-jira-client.ts | 30 +- .../runtime-jira-payload-stream.test.ts | 74 ++++ .../runtime/runtime-jira-payload-stream.ts | 94 +++++ src/shared/jira-payload-stream.ts | 12 + 32 files changed, 2474 insertions(+), 73 deletions(-) create mode 100644 src/main/jira/adf-markdown.test.ts create mode 100644 src/main/jira/adf-media-destination.ts create mode 100644 src/main/jira/attachment-discovery.ts create mode 100644 src/main/jira/attachment-image-cache.test.ts create mode 100644 src/main/jira/attachment-image-cache.ts create mode 100644 src/main/jira/attachment-images.test.ts create mode 100644 src/main/jira/attachment-images.ts create mode 100644 src/main/jira/attachment-meta.ts create mode 100644 src/renderer/src/components/sidebar/MarkdownImageLightbox.test.tsx create mode 100644 src/renderer/src/components/sidebar/MarkdownImageLightbox.tsx create mode 100644 src/renderer/src/runtime/runtime-jira-payload-stream.test.ts create mode 100644 src/renderer/src/runtime/runtime-jira-payload-stream.ts create mode 100644 src/shared/jira-payload-stream.ts diff --git a/src/main/jira/adf-markdown.test.ts b/src/main/jira/adf-markdown.test.ts new file mode 100644 index 000000000..ce6a160b5 --- /dev/null +++ b/src/main/jira/adf-markdown.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it, vi } from 'vitest' +import { adfToMarkdownText, collectAdfMediaAttrs } from './adf-markdown' +import { escapeMarkdownLinkDestination } from './adf-media-destination' + +describe('adfToMarkdownText media', () => { + it('keeps a placeholder when media cannot be resolved', () => { + const markdown = adfToMarkdownText({ + type: 'doc', + version: 1, + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Before' }] }, + { + type: 'mediaSingle', + attrs: { layout: 'center' }, + content: [ + { + type: 'media', + attrs: { + id: 'media-uuid-1', + type: 'file', + collection: 'contentId-1', + alt: 'screenshot.png' + } + } + ] + }, + { type: 'paragraph', content: [{ type: 'text', text: 'After' }] } + ] + }) + + expect(markdown).toBe('Before\n\n*[screenshot.png]*\n\nAfter') + }) + + it('uses the media resolver for file media nodes', () => { + const resolveMedia = vi.fn(() => '![shot.png](data:image/png;base64,abc)') + const markdown = adfToMarkdownText( + { + type: 'doc', + version: 1, + content: [ + { + type: 'mediaSingle', + content: [ + { + type: 'media', + attrs: { id: 'media-1', type: 'file', alt: 'shot.png' } + } + ] + } + ] + }, + { resolveMedia } + ) + + expect(resolveMedia).toHaveBeenCalledWith({ + id: 'media-1', + url: undefined, + alt: 'shot.png', + type: 'file' + }) + expect(markdown).toBe('![shot.png](data:image/png;base64,abc)') + }) + + it('renders external media URLs without a resolver', () => { + const markdown = adfToMarkdownText({ + type: 'doc', + version: 1, + content: [ + { + type: 'mediaSingle', + content: [ + { + type: 'media', + attrs: { + type: 'external', + url: 'https://example.com/diagram.png', + alt: 'diagram' + } + } + ] + } + ] + }) + + expect(markdown).toBe('![diagram](https://example.com/diagram.png)') + }) + + it('renders mediaInline inside paragraphs', () => { + const markdown = adfToMarkdownText( + { + type: 'doc', + version: 1, + content: [ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'See ' }, + { + type: 'mediaInline', + attrs: { id: 'inline-1', type: 'file', alt: 'icon.png' } + } + ] + } + ] + }, + { + resolveMedia: () => '![icon.png](data:image/png;base64,xyz)' + } + ) + + expect(markdown).toBe('See ![icon.png](data:image/png;base64,xyz)') + }) + + it('escapes markdown-hostile external media destinations', () => { + const hostile = 'https://cdn.example/x?a=1)![z](https://evil.example/y' + // Pin: encodeURI alone does not encode ) + expect(encodeURI(hostile)).toContain(')') + const safe = escapeMarkdownLinkDestination(hostile) + expect(safe).not.toBeNull() + expect(safe).not.toContain(')') + expect(safe).toContain('%29') + + const markdown = adfToMarkdownText({ + type: 'doc', + version: 1, + content: [ + { + type: 'mediaSingle', + content: [{ type: 'media', attrs: { type: 'external', url: hostile, alt: 'Image' } }] + } + ] + }) + expect(markdown).toContain('%29') + expect(markdown).not.toContain('](https://evil') + }) + + it('preserves existing percent-escapes when encoding destinations', () => { + const url = 'https://cdn.example/path%20with%20space.png' + expect(escapeMarkdownLinkDestination(url)).toBe(url) + }) + + it('collects media attrs in document order', () => { + const attrs = collectAdfMediaAttrs({ + type: 'doc', + content: [ + { type: 'media', attrs: { id: 'a', alt: 'one.png' } }, + { + type: 'paragraph', + content: [{ type: 'mediaInline', attrs: { id: 'b', url: 'https://x.example/y.png' } }] + } + ] + }) + expect(attrs).toEqual([ + { id: 'a', alt: 'one.png' }, + { id: 'b', url: 'https://x.example/y.png' } + ]) + }) +}) diff --git a/src/main/jira/adf-markdown.ts b/src/main/jira/adf-markdown.ts index 2c4eea30e..2706e6469 100644 --- a/src/main/jira/adf-markdown.ts +++ b/src/main/jira/adf-markdown.ts @@ -1,3 +1,5 @@ +import { escapeMarkdownLinkDestination } from './adf-media-destination' + type JiraAdfRecord = Record type MarkdownBlock = { @@ -5,6 +7,20 @@ type MarkdownBlock = { text: string } +export type JiraAdfMediaAttrs = { + id?: string + url?: string + alt?: string + type?: string +} + +/** Returns markdown for a media node (usually `![alt](src)`), or null to fall back. */ +export type JiraAdfMediaResolver = (attrs: JiraAdfMediaAttrs) => string | null + +export type AdfToMarkdownOptions = { + resolveMedia?: JiraAdfMediaResolver +} + function asRecord(value: unknown): JiraAdfRecord { return value && typeof value === 'object' ? (value as JiraAdfRecord) : {} } @@ -41,7 +57,72 @@ function headingLevel(value: unknown): number { return Math.min(Math.max(positiveInteger(value, 1), 1), 6) } -function renderInline(node: unknown): string { +export function escapeMarkdownAlt(text: string): string { + return text.replace(/[[\]]/g, '') +} + +function mediaAttrsFromRecord(record: JiraAdfRecord): JiraAdfMediaAttrs { + const attrs = asRecord(record.attrs) + return { + id: asString(attrs.id) || undefined, + url: asString(attrs.url) || undefined, + alt: asString(attrs.alt) || asString(attrs.name) || undefined, + type: asString(attrs.type) || undefined + } +} + +export function unresolvedMediaPlaceholder(attrs: JiraAdfMediaAttrs): string { + const label = escapeMarkdownAlt(attrs.alt?.trim() || 'Image') + // Why: keep a visible marker when media cannot be downloaded so screenshots + // are not silently dropped from the issue body. + return `*[${label}]*` +} + +/** Collect media attrs in document order (read-only; separate from adfToMarkdownText). */ +export function collectAdfMediaAttrs(value: unknown): JiraAdfMediaAttrs[] { + const collected: JiraAdfMediaAttrs[] = [] + + const walk = (node: unknown): void => { + if (!node || typeof node !== 'object') { + return + } + if (Array.isArray(node)) { + for (const child of node) { + walk(child) + } + return + } + const record = node as JiraAdfRecord + if (record.type === 'media' || record.type === 'mediaInline') { + collected.push(mediaAttrsFromRecord(record)) + } + walk(record.content) + } + + walk(value) + return collected +} + +function renderMediaMarkdown( + record: JiraAdfRecord, + options: AdfToMarkdownOptions | undefined +): string { + const attrs = mediaAttrsFromRecord(record) + const resolved = options?.resolveMedia?.(attrs) + if (resolved) { + return resolved + } + if (attrs.url && /^https?:\/\//i.test(attrs.url)) { + const safeUrl = escapeMarkdownLinkDestination(attrs.url) + if (!safeUrl) { + return unresolvedMediaPlaceholder(attrs) + } + return `![${escapeMarkdownAlt(attrs.alt?.trim() || 'Image')}](${safeUrl})` + } + return unresolvedMediaPlaceholder(attrs) +} + +function renderInline(node: unknown, options?: AdfToMarkdownOptions): string { if (!node) { return '' } @@ -49,7 +130,7 @@ function renderInline(node: unknown): string { return node } if (Array.isArray(node)) { - return node.map(renderInline).join('') + return node.map((child) => renderInline(child, options)).join('') } if (typeof node !== 'object') { return '' @@ -62,6 +143,11 @@ function renderInline(node: unknown): string { if (record.type === 'hardBreak') { return '\n' } + // Why: Jira pastes screenshots as media/mediaInline ADF nodes; without this + // branch they collapse to empty strings and disappear from the UI. + if (record.type === 'media' || record.type === 'mediaInline') { + return renderMediaMarkdown(record, options) + } const attrs = asRecord(record.attrs) const fallbackText = asString(attrs.text) || asString(attrs.shortName) || asString(attrs.url) @@ -69,7 +155,7 @@ function renderInline(node: unknown): string { return fallbackText } - return renderInline(record.content) + return renderInline(record.content, options) } function joinBlocks(blocks: MarkdownBlock[]): string { @@ -79,14 +165,14 @@ function joinBlocks(blocks: MarkdownBlock[]): string { .join('\n\n') } -function renderBlocks(content: unknown): MarkdownBlock[] { +function renderBlocks(content: unknown, options?: AdfToMarkdownOptions): MarkdownBlock[] { return asArray(content) - .map(renderBlock) + .map((node) => renderBlock(node, options)) .filter((block) => block.text.length > 0) } -function renderListItem(node: unknown, prefix: string): string { - const blocks = renderBlocks(asRecord(node).content) +function renderListItem(node: unknown, prefix: string, options?: AdfToMarkdownOptions): string { + const blocks = renderBlocks(asRecord(node).content, options) if (blocks.length === 0) { return prefix.trimEnd() } @@ -114,20 +200,24 @@ function renderListItem(node: unknown, prefix: string): string { return lines.join('\n') } -function renderList(record: JiraAdfRecord, ordered: boolean): string { +function renderList( + record: JiraAdfRecord, + ordered: boolean, + options?: AdfToMarkdownOptions +): string { const start = ordered ? positiveInteger(asRecord(record.attrs).order, 1) : 1 return asArray(record.content) - .map((item, index) => renderListItem(item, ordered ? `${start + index}. ` : '- ')) + .map((item, index) => renderListItem(item, ordered ? `${start + index}. ` : '- ', options)) .join('\n') } -function renderCodeBlock(record: JiraAdfRecord): MarkdownBlock { - const text = renderInline(record.content).replace(/\n$/, '') +function renderCodeBlock(record: JiraAdfRecord, options?: AdfToMarkdownOptions): MarkdownBlock { + const text = renderInline(record.content, options).replace(/\n$/, '') return { kind: 'block', text: ['```', text, '```'].join('\n') } } -function renderBlockquote(record: JiraAdfRecord): MarkdownBlock { - const text = joinBlocks(renderBlocks(record.content)) +function renderBlockquote(record: JiraAdfRecord, options?: AdfToMarkdownOptions): MarkdownBlock { + const text = joinBlocks(renderBlocks(record.content, options)) return { kind: 'block', text: text @@ -137,12 +227,12 @@ function renderBlockquote(record: JiraAdfRecord): MarkdownBlock { } } -function renderBlock(node: unknown): MarkdownBlock { +function renderBlock(node: unknown, options?: AdfToMarkdownOptions): MarkdownBlock { if (typeof node === 'string') { return { kind: 'block', text: node } } if (Array.isArray(node)) { - return { kind: 'block', text: joinBlocks(renderBlocks(node)) } + return { kind: 'block', text: joinBlocks(renderBlocks(node, options)) } } if (!node || typeof node !== 'object') { return { kind: 'block', text: '' } @@ -151,41 +241,54 @@ function renderBlock(node: unknown): MarkdownBlock { const record = node as JiraAdfRecord const type = asString(record.type) if (type === 'doc') { - return { kind: 'block', text: joinBlocks(renderBlocks(record.content)) } + return { kind: 'block', text: joinBlocks(renderBlocks(record.content, options)) } } if (type === 'paragraph') { - return { kind: 'block', text: renderInline(record.content) } + return { kind: 'block', text: renderInline(record.content, options) } } if (type === 'heading') { const prefix = '#'.repeat(headingLevel(asRecord(record.attrs).level)) - return { kind: 'block', text: `${prefix} ${renderInline(record.content).trim()}`.trim() } + return { + kind: 'block', + text: `${prefix} ${renderInline(record.content, options).trim()}`.trim() + } } if (type === 'bulletList') { // Why: Orca renders Jira bodies as Markdown, so ADF list containers need // concrete list markers instead of newline-only flattened text. - return { kind: 'list', text: renderList(record, false) } + return { kind: 'list', text: renderList(record, false, options) } } if (type === 'orderedList') { - return { kind: 'list', text: renderList(record, true) } + return { kind: 'list', text: renderList(record, true, options) } } if (type === 'listItem') { - return { kind: 'list', text: renderListItem(record, '- ') } + return { kind: 'list', text: renderListItem(record, '- ', options) } } if (type === 'codeBlock') { - return renderCodeBlock(record) + return renderCodeBlock(record, options) } if (type === 'blockquote') { - return renderBlockquote(record) + return renderBlockquote(record, options) } if (type === 'rule') { return { kind: 'block', text: '---' } } + if (type === 'mediaSingle' || type === 'mediaGroup') { + const mediaMarkdown = joinBlocks(renderBlocks(record.content, options)) + return { kind: 'block', text: mediaMarkdown } + } + if (type === 'media' || type === 'mediaInline') { + return { kind: 'block', text: renderMediaMarkdown(record, options) } + } - return { kind: 'block', text: joinBlocks(renderBlocks(record.content)) || renderInline(record) } + return { + kind: 'block', + text: joinBlocks(renderBlocks(record.content, options)) || renderInline(record, options) + } } -export function adfToMarkdownText(value: unknown): string { - return renderBlock(value) +export function adfToMarkdownText(value: unknown, options?: AdfToMarkdownOptions): string { + return renderBlock(value, options) .text.replace(/[ \t]+\n/g, '\n') .replace(/\n{3,}/g, '\n\n') .trim() diff --git a/src/main/jira/adf-media-destination.ts b/src/main/jira/adf-media-destination.ts new file mode 100644 index 000000000..e92cce030 --- /dev/null +++ b/src/main/jira/adf-media-destination.ts @@ -0,0 +1,56 @@ +// Why: markdown image destinations close at the first unescaped `)`. encodeURI +// leaves `()` alone, so Jira-controlled external URLs must be destination-safe. + +const MARKDOWN_DESTINATION_HOSTILE = new Set(['(', ')', '[', ']', '<', '>', '"', "'", '`', '\\']) + +function isMarkdownDestinationHostile(char: string): boolean { + if (MARKDOWN_DESTINATION_HOSTILE.has(char)) { + return true + } + const code = char.charCodeAt(0) + return code <= 0x20 +} + +function percentEncodeUtf8Char(char: string): string { + return Array.from( + new TextEncoder().encode(char), + (byte) => `%${byte.toString(16).toUpperCase().padStart(2, '0')}` + ).join('') +} + +/** Percent-encode markdown-hostile destination chars without double-encoding `%HH`. */ +export function escapeMarkdownLinkDestination(url: string): string | null { + if (!/^https?:\/\//i.test(url)) { + return null + } + + let encoded = '' + for (let i = 0; i < url.length; ) { + const char = url[i] ?? '' + if (char === '%') { + const hex = url.slice(i + 1, i + 3) + if (/^[0-9a-fA-F]{2}$/.test(hex)) { + encoded += `%${hex}` + i += 3 + continue + } + encoded += '%25' + i += 1 + continue + } + if (isMarkdownDestinationHostile(char)) { + encoded += percentEncodeUtf8Char(char) + i += 1 + continue + } + encoded += char + i += 1 + } + + for (const char of encoded) { + if (isMarkdownDestinationHostile(char)) { + return null + } + } + return encoded +} diff --git a/src/main/jira/attachment-discovery.ts b/src/main/jira/attachment-discovery.ts new file mode 100644 index 000000000..945acbac2 --- /dev/null +++ b/src/main/jira/attachment-discovery.ts @@ -0,0 +1,99 @@ +import type { JiraAdfMediaAttrs } from './adf-markdown' +import { MAX_IMAGES, parseImageAttachmentMetas } from './attachment-meta' + +/** + * Pull attachment content IDs from Jira rendered HTML in document order. + * Why: thumbnail paths use the same numeric attachment id as content URLs. + */ +export function extractAttachmentContentIdsFromHtml(html: string | undefined | null): string[] { + if (!html) { + return [] + } + const ids: string[] = [] + const seen = new Set() + // content, secure/attachment, thumbnail, and rest thumbnail forms + const pattern = + /\/(?:rest\/api\/\d+\/attachment\/(?:content|thumbnail)|secure\/(?:attachment|thumbnail))\/(\d+)(?:\/|\b|"|'|\?)/gi + let match: RegExpExecArray | null + while ((match = pattern.exec(html)) !== null) { + const id = match[1] + if (!id || seen.has(id)) { + continue + } + seen.add(id) + ids.push(id) + } + return ids +} + +/** Prefer attachment-needing media (no external https URL) for discovery fallback. */ +export function selectPreferredAttachmentIds(args: { + renderedHtmlIds: string[] + attachmentField: unknown + mediaAttrs: readonly JiraAdfMediaAttrs[] +}): { preferredIds: string[]; fallbackRan: boolean; needCount: number } { + const needing = args.mediaAttrs.filter((attrs) => !(attrs.url && /^https?:\/\//i.test(attrs.url))) + const needCount = needing.length + if (needCount === 0) { + // Why: no attachment-needing ADF media — skip downloads (do not sweep HTML-only). + return { preferredIds: [], fallbackRan: false, needCount: 0 } + } + + const preferredIds = [...args.renderedHtmlIds] + const taken = new Set(preferredIds) + let fallbackRan = false + + if (needCount > preferredIds.length) { + const metas = parseImageAttachmentMetas(args.attachmentField) + // Why: multiple Jira screenshots often share image.png — assign next unused meta per node. + for (const node of needing) { + if (preferredIds.length >= MAX_IMAGES) { + break + } + const alt = (node.alt ?? '').trim() + if (!alt) { + continue + } + const altKey = alt.toLowerCase() + const meta = metas.find( + (candidate) => candidate.filename.toLowerCase() === altKey && !taken.has(candidate.id) + ) + if (!meta) { + continue + } + taken.add(meta.id) + preferredIds.push(meta.id) + fallbackRan = true + } + } + + return { + preferredIds: preferredIds.slice(0, MAX_IMAGES), + fallbackRan, + needCount + } +} + +export function warnIfMediaResolutionIncomplete(args: { + siteId: string + issueKey: string + needCount: number + preferredIdCount: number + resolvedCount: number + fallbackRan: boolean +}): void { + if (args.needCount <= 0) { + return + } + if (args.resolvedCount >= args.needCount) { + return + } + console.warn('[jira] inline image resolution incomplete', { + siteId: args.siteId, + issueKey: args.issueKey, + needCount: args.needCount, + preferredIdCount: args.preferredIdCount, + resolvedCount: args.resolvedCount, + fallbackRan: args.fallbackRan + }) +} diff --git a/src/main/jira/attachment-image-cache.test.ts b/src/main/jira/attachment-image-cache.test.ts new file mode 100644 index 000000000..4f213327c --- /dev/null +++ b/src/main/jira/attachment-image-cache.test.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + _getAttachmentImageCacheSize, + _resetAttachmentImageCache, + clearAttachmentImagesForSite, + getCachedAttachmentDataUrl, + loadAttachmentDataUrlWithCache, + setCachedAttachmentDataUrl +} from './attachment-image-cache' + +describe('attachment image cache', () => { + beforeEach(() => { + _resetAttachmentImageCache() + }) + + it('returns cached data urls and isolates sites', () => { + setCachedAttachmentDataUrl({ + siteId: 'a', + attachmentId: '1', + dataUrl: 'data:image/png;base64,AA==', + byteSize: 1 + }) + setCachedAttachmentDataUrl({ + siteId: 'b', + attachmentId: '1', + dataUrl: 'data:image/png;base64,BB==', + byteSize: 1 + }) + expect(getCachedAttachmentDataUrl('a', '1')).toBe('data:image/png;base64,AA==') + expect(getCachedAttachmentDataUrl('b', '1')).toBe('data:image/png;base64,BB==') + clearAttachmentImagesForSite('a') + expect(getCachedAttachmentDataUrl('a', '1')).toBeNull() + expect(getCachedAttachmentDataUrl('b', '1')).toBe('data:image/png;base64,BB==') + }) + + it('singleflights concurrent loads and does not cache failures', async () => { + let calls = 0 + let resolveLoad: (value: { dataUrl: string; byteSize: number } | null) => void = () => {} + const load = () => + new Promise<{ dataUrl: string; byteSize: number } | null>((resolve) => { + calls += 1 + resolveLoad = resolve + }) + + const p1 = loadAttachmentDataUrlWithCache({ siteId: 's', attachmentId: '1', load }) + const p2 = loadAttachmentDataUrlWithCache({ siteId: 's', attachmentId: '1', load }) + expect(calls).toBe(1) + resolveLoad(null) + expect(await p1).toBeNull() + expect(await p2).toBeNull() + expect(getCachedAttachmentDataUrl('s', '1')).toBeNull() + + const p3 = loadAttachmentDataUrlWithCache({ + siteId: 's', + attachmentId: '1', + load: async () => ({ dataUrl: 'data:image/png;base64,OK==', byteSize: 2 }) + }) + expect(await p3).toBe('data:image/png;base64,OK==') + expect(_getAttachmentImageCacheSize()).toBe(1) + }) +}) diff --git a/src/main/jira/attachment-image-cache.ts b/src/main/jira/attachment-image-cache.ts new file mode 100644 index 000000000..d1e0f42a5 --- /dev/null +++ b/src/main/jira/attachment-image-cache.ts @@ -0,0 +1,169 @@ +// Why: description and comments fetch attachments independently; comments also +// refetch after every post. Cache finished data URLs in main so the second path +// does not re-download or re-base64 the same attachment bytes. + +const CACHE_TTL_MS = 30 * 60_000 +const MAX_CACHE_ENTRIES = 96 +const MAX_CACHE_BYTES = 24 * 1024 * 1024 + +type CacheEntry = { + dataUrl: string + byteSize: number + storedAt: number +} + +const cache = new Map() +const inFlight = new Map>() +// Why: mid-flight downloads must not repopulate cache after disconnect/clearToken. +let cacheEpoch = 0 +const siteEpoch = new Map() + +function cacheKey(siteId: string, attachmentId: string): string { + return `${siteId}::${attachmentId}` +} + +function currentEpoch(siteId: string): number { + return (siteEpoch.get(siteId) ?? 0) + cacheEpoch +} + +function pruneExpired(now = Date.now()): void { + for (const [key, entry] of cache) { + if (now - entry.storedAt >= CACHE_TTL_MS) { + cache.delete(key) + } + } +} + +function totalCachedBytes(): number { + let total = 0 + for (const entry of cache.values()) { + total += entry.byteSize + } + return total +} + +function evictUntilWithinBounds(): void { + while (cache.size > MAX_CACHE_ENTRIES || totalCachedBytes() > MAX_CACHE_BYTES) { + const oldestKey = cache.keys().next().value + if (oldestKey === undefined) { + break + } + cache.delete(oldestKey) + } +} + +export function getCachedAttachmentDataUrl(siteId: string, attachmentId: string): string | null { + pruneExpired() + const key = cacheKey(siteId, attachmentId) + const entry = cache.get(key) + if (!entry) { + return null + } + if (Date.now() - entry.storedAt >= CACHE_TTL_MS) { + cache.delete(key) + return null + } + // Why: delete-then-set refreshes insertion order for LRU-style eviction. + cache.delete(key) + cache.set(key, entry) + return entry.dataUrl +} + +export function setCachedAttachmentDataUrl(args: { + siteId: string + attachmentId: string + dataUrl: string + byteSize: number +}): void { + pruneExpired() + const key = cacheKey(args.siteId, args.attachmentId) + const entry: CacheEntry = { + dataUrl: args.dataUrl, + byteSize: args.byteSize, + storedAt: Date.now() + } + cache.delete(key) + cache.set(key, entry) + evictUntilWithinBounds() +} + +/** + * Singleflight loader: concurrent cold misses for the same attachment share one + * download. Failed loads are not cached so a later retry can succeed. + */ +export async function loadAttachmentDataUrlWithCache(args: { + siteId: string + attachmentId: string + load: () => Promise<{ dataUrl: string; byteSize: number } | null> +}): Promise { + const cached = getCachedAttachmentDataUrl(args.siteId, args.attachmentId) + if (cached) { + return cached + } + + const key = cacheKey(args.siteId, args.attachmentId) + const existing = inFlight.get(key) + if (existing) { + return existing + } + + const epochAtStart = currentEpoch(args.siteId) + const promise = (async (): Promise => { + try { + const loaded = await args.load() + if (!loaded) { + return null + } + // Why: return bytes to the waiter but skip cache if site was cleared mid-flight. + if (currentEpoch(args.siteId) === epochAtStart) { + setCachedAttachmentDataUrl({ + siteId: args.siteId, + attachmentId: args.attachmentId, + dataUrl: loaded.dataUrl, + byteSize: loaded.byteSize + }) + } + return loaded.dataUrl + } finally { + inFlight.delete(key) + } + })() + + inFlight.set(key, promise) + return promise +} + +export function clearAttachmentImagesForSite(siteId?: string): void { + if (siteId == null || siteId === '') { + cache.clear() + inFlight.clear() + cacheEpoch += 1 + siteEpoch.clear() + return + } + siteEpoch.set(siteId, (siteEpoch.get(siteId) ?? 0) + 1) + const prefix = `${siteId}::` + for (const key of cache.keys()) { + if (key.startsWith(prefix)) { + cache.delete(key) + } + } + for (const key of inFlight.keys()) { + if (key.startsWith(prefix)) { + inFlight.delete(key) + } + } +} + +/** @internal — test-only */ +export function _resetAttachmentImageCache(): void { + cache.clear() + inFlight.clear() + cacheEpoch = 0 + siteEpoch.clear() +} + +/** @internal — test-only */ +export function _getAttachmentImageCacheSize(): number { + return cache.size +} diff --git a/src/main/jira/attachment-images.test.ts b/src/main/jira/attachment-images.test.ts new file mode 100644 index 000000000..e160f6909 --- /dev/null +++ b/src/main/jira/attachment-images.test.ts @@ -0,0 +1,351 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { JiraClientForSite } from './client' + +const { jiraRequestBinaryMock } = vi.hoisted(() => ({ + jiraRequestBinaryMock: vi.fn() +})) + +vi.mock('./client', () => ({ + jiraRequestBinary: (...args: unknown[]) => jiraRequestBinaryMock(...args), + apiBasePath: (site: { authType?: string }) => + site.authType === 'server' ? '/rest/api/2' : '/rest/api/3', + JiraApiError: class JiraApiError extends Error { + status: number | null + constructor(message: string, status: number | null = null) { + super(message) + this.status = status + } + } +})) + +function makeEntry(): JiraClientForSite { + return { + site: { + id: 'site-1', + siteUrl: 'https://example.atlassian.net', + email: 'ada@example.com', + displayName: 'Example Jira', + accountId: 'account-1' + }, + authorization: 'Basic token' + } +} + +describe('attachment image helpers', () => { + beforeEach(async () => { + jiraRequestBinaryMock.mockReset() + const { _resetAttachmentImageCache } = await import('./attachment-image-cache') + _resetAttachmentImageCache() + }) + + it('extracts attachment content ids from rendered HTML in order', async () => { + const { extractAttachmentContentIdsFromHtml } = await import('./attachment-discovery') + const ids = extractAttachmentContentIdsFromHtml(` +

intro

+ + + + `) + expect(ids).toEqual(['101', '202']) + }) + + it('extracts thumbnail attachment ids', async () => { + const { extractAttachmentContentIdsFromHtml } = await import('./attachment-discovery') + expect( + extractAttachmentContentIdsFromHtml( + '' + ) + ).toEqual(['10001']) + expect( + extractAttachmentContentIdsFromHtml( + '' + ) + ).toEqual(['10002']) + }) + + it('downloads image attachments and builds a media resolver', async () => { + const pngBytes = Uint8Array.from([137, 80, 78, 71]) + jiraRequestBinaryMock.mockResolvedValue({ + data: pngBytes.buffer, + contentType: 'image/png' + }) + + const { createMediaMarkdownResolver, loadIssueImageAttachments } = + await import('./attachment-images') + + const images = await loadIssueImageAttachments( + makeEntry(), + [ + { + id: '101', + filename: 'shot.png', + mimeType: 'image/png', + size: 4 + }, + { + id: '202', + filename: 'notes.txt', + mimeType: 'text/plain', + size: 12 + } + ], + ['101'] + ) + + expect(images).toHaveLength(1) + expect(images[0]?.id).toBe('101') + expect(images[0]?.dataUrl.startsWith('data:image/png;base64,')).toBe(true) + expect(jiraRequestBinaryMock).toHaveBeenCalledWith( + expect.anything(), + 'https://example.atlassian.net/rest/api/3/attachment/content/101?redirect=false' + ) + + const resolve = createMediaMarkdownResolver(images, ['101']) + const resolved = `![shot.png](${images[0]?.dataUrl})` + expect(resolve({ id: 'media-uuid', type: 'file', alt: 'shot.png' })).toBe(resolved) + expect(resolve({ id: 'media-uuid', type: 'file' })).toBe(resolved) + expect(resolve({ id: 'media-uuid-2', type: 'file' })).toBeNull() + }) + + it('pairs shared alt filenames to distinct attachments then falls through', async () => { + const { createMediaMarkdownResolver } = await import('./attachment-images') + const images = [ + { + id: '1', + filename: 'image.png', + mimeType: 'image/png', + byteSize: 1, + dataUrl: 'data:image/png;base64,AA==' + }, + { + id: '2', + filename: 'other.png', + mimeType: 'image/png', + byteSize: 1, + dataUrl: 'data:image/png;base64,BB==' + } + ] + const resolve = createMediaMarkdownResolver(images, ['1', '2']) + expect(resolve({ id: 'm1', alt: 'image.png' })).toBe('![image.png](data:image/png;base64,AA==)') + // Exhausted filename match must not re-emit image 1 + expect(resolve({ id: 'm2', alt: 'image.png' })).toBe('![other.png](data:image/png;base64,BB==)') + }) + + it('does not re-emit an already consumed attachment for a third shared-alt node', async () => { + const { createMediaMarkdownResolver } = await import('./attachment-images') + const images = [ + { + id: '1', + filename: 'image.png', + mimeType: 'image/png', + byteSize: 1, + dataUrl: 'data:image/png;base64,AA==' + }, + { + id: '2', + filename: 'image.png', + mimeType: 'image/png', + byteSize: 1, + dataUrl: 'data:image/png;base64,BB==' + } + ] + const resolve = createMediaMarkdownResolver(images, ['1', '2']) + expect(resolve({ id: 'm1', alt: 'image.png' })).toContain('AA==') + expect(resolve({ id: 'm2', alt: 'image.png' })).toContain('BB==') + expect(resolve({ id: 'm3', alt: 'image.png' })).toBeNull() + }) + + it('escapes hostile external media URLs instead of injecting markdown', async () => { + const { createMediaMarkdownResolver } = await import('./attachment-images') + const resolve = createMediaMarkdownResolver([], []) + const hostile = 'https://evil.example/x?a=1)![z](javascript:alert(1))' + const out = resolve({ url: hostile, alt: 'Image' }) + expect(out).not.toContain('](javascript:') + expect(out).toMatch(/^!\[[^\]]*\]\(https:\/\/evil\.example/) + // encodeURI leaves ) unencoded — pin the bug class + expect(encodeURI(hostile)).toContain(')') + expect(out).not.toBe(`![Image](${hostile})`) + }) + + it('returns placeholder for external URLs that remain hostile after encode', async () => { + const { createMediaMarkdownResolver } = await import('./attachment-images') + const resolve = createMediaMarkdownResolver([], []) + // non-http rejected + expect(resolve({ url: 'javascript:alert(1)', alt: 'x' })).toBe('*[x]*') + }) + + it('selects preferred ids via Option A filename fallback without sweeping all attachments', async () => { + const { selectPreferredAttachmentIds } = await import('./attachment-discovery') + const attachments = [ + { id: '1', filename: 'a.png', mimeType: 'image/png', size: 1 }, + { id: '2', filename: 'b.png', mimeType: 'image/png', size: 1 }, + { id: '3', filename: 'unrelated.png', mimeType: 'image/png', size: 1 } + ] + const selection = selectPreferredAttachmentIds({ + renderedHtmlIds: [], + attachmentField: attachments, + mediaAttrs: [{ alt: 'a.png' }, { alt: 'b.png' }] + }) + expect(selection.preferredIds).toEqual(['1', '2']) + expect(selection.fallbackRan).toBe(true) + expect(selection.needCount).toBe(2) + + const noMedia = selectPreferredAttachmentIds({ + renderedHtmlIds: [], + attachmentField: attachments, + mediaAttrs: [] + }) + expect(noMedia.preferredIds).toEqual([]) + expect(noMedia.needCount).toBe(0) + }) + + it('Option A unions multiple same-filename attachments for repeated alts', async () => { + const { selectPreferredAttachmentIds } = await import('./attachment-discovery') + const attachments = [ + { id: '1', filename: 'image.png', mimeType: 'image/png', size: 1 }, + { id: '2', filename: 'image.png', mimeType: 'image/png', size: 1 } + ] + const zeroHtml = selectPreferredAttachmentIds({ + renderedHtmlIds: [], + attachmentField: attachments, + mediaAttrs: [{ alt: 'image.png' }, { alt: 'image.png' }] + }) + expect(zeroHtml.preferredIds).toEqual(['1', '2']) + expect(zeroHtml.fallbackRan).toBe(true) + + const partialHtml = selectPreferredAttachmentIds({ + renderedHtmlIds: ['1'], + attachmentField: attachments, + mediaAttrs: [{ alt: 'image.png' }, { alt: 'image.png' }] + }) + expect(partialHtml.preferredIds).toEqual(['1', '2']) + }) + + it('downloads only referenced attachments after prioritizing the complete metadata list', async () => { + jiraRequestBinaryMock.mockResolvedValue({ + data: Uint8Array.from([1]).buffer, + contentType: 'image/png' + }) + const { loadIssueImageAttachments } = await import('./attachment-images') + const attachments = Array.from({ length: 13 }, (_, index) => ({ + id: String(index + 1), + filename: `${index + 1}.png`, + mimeType: 'image/png', + size: 1 + })) + + const images = await loadIssueImageAttachments(makeEntry(), attachments, ['13']) + + expect(images.map((image) => image.id)).toEqual(['13']) + expect(jiraRequestBinaryMock).toHaveBeenCalledTimes(1) + expect(jiraRequestBinaryMock).toHaveBeenCalledWith( + expect.anything(), + 'https://example.atlassian.net/rest/api/3/attachment/content/13?redirect=false' + ) + }) + + it('does not download attachments when rendered content references none', async () => { + const { loadIssueImageAttachments } = await import('./attachment-images') + + await expect( + loadIssueImageAttachments( + makeEntry(), + [{ id: '1', filename: 'unrelated.png', mimeType: 'image/png', size: 1 }], + [] + ) + ).resolves.toEqual([]) + expect(jiraRequestBinaryMock).not.toHaveBeenCalled() + }) + + it('uses the attachment content URI supplied by self-hosted Jira', async () => { + jiraRequestBinaryMock.mockResolvedValue({ + data: Uint8Array.from([1]).buffer, + contentType: 'image/png' + }) + const entry = makeEntry() + entry.site = { + ...entry.site, + siteUrl: 'https://jira.example.com/jira', + authType: 'server' + } + const { loadIssueImageAttachments } = await import('./attachment-images') + + await loadIssueImageAttachments( + entry, + [ + { + id: '42', + filename: 'server.png', + mimeType: 'image/png', + size: 1, + content: 'https://jira.example.com/jira/secure/attachment/42/server.png' + } + ], + ['42'] + ) + + expect(jiraRequestBinaryMock).toHaveBeenCalledWith( + expect.anything(), + 'https://jira.example.com/jira/secure/attachment/42/server.png' + ) + }) + + it('skips oversized and non-image attachments', async () => { + const { parseImageAttachmentMetas } = await import('./attachment-meta') + expect( + parseImageAttachmentMetas([ + { id: '1', filename: 'big.png', mimeType: 'image/png', size: 20 * 1024 * 1024 }, + { id: '2', filename: 'icon.svg', mimeType: 'image/svg+xml', size: 100 }, + { id: '3', filename: 'ok.jpg', mimeType: 'image/jpeg', size: 100 } + ]) + ).toEqual([{ id: '3', filename: 'ok.jpg', mimeType: 'image/jpeg', size: 100 }]) + }) + + it('serves a second load of the same attachment from cache', async () => { + jiraRequestBinaryMock.mockResolvedValue({ + data: Uint8Array.from([1, 2, 3]).buffer, + contentType: 'image/png' + }) + const { loadIssueImageAttachments } = await import('./attachment-images') + const entry = makeEntry() + const field = [{ id: '9', filename: 'c.png', mimeType: 'image/png', size: 3 }] + await loadIssueImageAttachments(entry, field, ['9']) + await loadIssueImageAttachments(entry, field, ['9']) + expect(jiraRequestBinaryMock).toHaveBeenCalledTimes(1) + }) + + it('singleflights concurrent cold misses for the same attachment', async () => { + let resolveDownload: (value: { data: ArrayBuffer; contentType: string }) => void = () => {} + jiraRequestBinaryMock.mockImplementation( + () => + new Promise((resolve) => { + resolveDownload = resolve + }) + ) + const { loadIssueImageAttachments } = await import('./attachment-images') + const entry = makeEntry() + const field = [{ id: '7', filename: 's.png', mimeType: 'image/png', size: 1 }] + const p1 = loadIssueImageAttachments(entry, field, ['7']) + const p2 = loadIssueImageAttachments(entry, field, ['7']) + resolveDownload({ data: Uint8Array.from([9]).buffer, contentType: 'image/png' }) + const [a, b] = await Promise.all([p1, p2]) + expect(a).toHaveLength(1) + expect(b).toHaveLength(1) + expect(jiraRequestBinaryMock).toHaveBeenCalledTimes(1) + }) + + it('warns when media resolution is incomplete', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const { warnIfMediaResolutionIncomplete } = await import('./attachment-discovery') + warnIfMediaResolutionIncomplete({ + siteId: 's', + issueKey: 'ABC-1', + needCount: 2, + preferredIdCount: 1, + resolvedCount: 0, + fallbackRan: true + }) + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) +}) diff --git a/src/main/jira/attachment-images.ts b/src/main/jira/attachment-images.ts new file mode 100644 index 000000000..ce2e682b8 --- /dev/null +++ b/src/main/jira/attachment-images.ts @@ -0,0 +1,252 @@ +import type { JiraClientForSite } from './client' +import { JiraApiError, apiBasePath, jiraRequestBinary } from './client' +import type { JiraAdfMediaAttrs, JiraAdfMediaResolver } from './adf-markdown' +import { escapeMarkdownAlt, unresolvedMediaPlaceholder } from './adf-markdown' +import { escapeMarkdownLinkDestination } from './adf-media-destination' +import { loadAttachmentDataUrlWithCache } from './attachment-image-cache' +import { + MAX_IMAGE_BYTES, + MAX_IMAGES, + MAX_TOTAL_IMAGE_BYTES, + parseImageAttachmentMetas, + isImageMimeType, + type AttachmentMeta +} from './attachment-meta' +import { mapWithConcurrency } from '../../shared/map-with-concurrency' + +const DOWNLOAD_CONCURRENCY = 3 + +export type JiraImageAttachment = { + id: string + filename: string + mimeType: string + byteSize: number + dataUrl: string +} + +async function downloadImageAttachment( + client: JiraClientForSite, + meta: AttachmentMeta +): Promise { + if (!meta.contentUrl && client.site.authType === 'server') { + // Server/DC exposes attachment bytes through the metadata-provided content URI. + return null + } + + const dataUrl = await loadAttachmentDataUrlWithCache({ + siteId: client.site.id, + attachmentId: meta.id, + load: async () => { + try { + const contentUrl = meta.contentUrl + ? new URL(meta.contentUrl, `${client.site.siteUrl}/`) + : // Why: Cloud fallback uses apiBasePath so Server sites that lack contentUrl + // still hit /rest/api/2 if ever called; Server still requires content metadata. + new URL( + `${apiBasePath(client.site)}/attachment/content/${encodeURIComponent(meta.id)}`, + client.site.siteUrl + ) + if (/\/rest\/api\/(?:2|3)\/attachment\/content\/[^/]+$/i.test(contentUrl.pathname)) { + contentUrl.searchParams.set('redirect', 'false') + } + const binary = await jiraRequestBinary(client, contentUrl.toString()) + if (binary.data.byteLength === 0 || binary.data.byteLength > MAX_IMAGE_BYTES) { + return null + } + const contentType = binary.contentType.split(';')[0]?.trim() || meta.mimeType + if (!isImageMimeType(contentType) && !isImageMimeType(meta.mimeType)) { + return null + } + const mime = isImageMimeType(contentType) ? contentType : meta.mimeType + const base64 = Buffer.from(binary.data).toString('base64') + return { + dataUrl: `data:${mime};base64,${base64}`, + byteSize: binary.data.byteLength + } + } catch (error) { + // Why: one bad attachment should not blank the whole issue description. + if (error instanceof JiraApiError && error.status === 404) { + return null + } + console.warn('[jira] attachment image download failed:', meta.id, error) + return null + } + } + }) + + if (!dataUrl) { + return null + } + + const mimeMatch = /^data:([^;]+);base64,/.exec(dataUrl) + const mime = mimeMatch?.[1] || meta.mimeType + // Approximate byte size from base64 payload when served from cache. + const base64Part = dataUrl.includes(',') ? dataUrl.slice(dataUrl.indexOf(',') + 1) : '' + const byteSize = Math.floor((base64Part.length * 3) / 4) + + return { + id: meta.id, + filename: meta.filename, + mimeType: mime, + byteSize, + dataUrl + } +} + +export async function loadIssueImageAttachments( + client: JiraClientForSite, + attachmentField: unknown, + preferredIds: string[] = [] +): Promise { + const metas = parseImageAttachmentMetas(attachmentField) + if (metas.length === 0 || preferredIds.length === 0) { + return [] + } + + const byId = new Map(metas.map((meta) => [meta.id, meta])) + const ordered: AttachmentMeta[] = [] + const used = new Set() + + for (const id of preferredIds) { + const meta = byId.get(id) + if (meta && !used.has(meta.id)) { + ordered.push(meta) + used.add(meta.id) + } + } + + // Why: pre-select by declared size so concurrent downloads do not fetch bodies + // that will be dropped by the total budget after completion. + const toDownload: AttachmentMeta[] = [] + let plannedBytes = 0 + for (const meta of ordered.slice(0, MAX_IMAGES)) { + if (meta.size > 0 && plannedBytes + meta.size > MAX_TOTAL_IMAGE_BYTES) { + continue + } + toDownload.push(meta) + if (meta.size > 0) { + plannedBytes += meta.size + } + } + + const downloaded = await mapWithConcurrency(toDownload, DOWNLOAD_CONCURRENCY, (meta) => + downloadImageAttachment(client, meta) + ) + + const images: JiraImageAttachment[] = [] + let totalBytes = 0 + for (const image of downloaded) { + if (!image) { + continue + } + if (totalBytes + image.byteSize > MAX_TOTAL_IMAGE_BYTES) { + continue + } + totalBytes += image.byteSize + images.push(image) + } + return images +} + +export type MediaResolutionStats = { + /** Attachment-needing media nodes that successfully resolved to a data: image. */ + attachmentResolvedCount: number +} + +export function createMediaMarkdownResolver( + images: readonly JiraImageAttachment[], + preferredAttachmentIds: readonly string[] = [], + stats?: MediaResolutionStats +): JiraAdfMediaResolver { + const byId = new Map(images.map((image) => [image.id, image])) + const byFilename = new Map() + const resolvedByMediaId = new Map() + for (const image of images) { + const key = image.filename.toLowerCase() + const list = byFilename.get(key) ?? [] + list.push(image) + byFilename.set(key, list) + } + + // Prefer document-order attachment IDs from rendered HTML, then remaining images. + const queue: JiraImageAttachment[] = [] + const queued = new Set() + for (const id of preferredAttachmentIds) { + const image = byId.get(id) + if (image && !queued.has(image.id)) { + queue.push(image) + queued.add(image.id) + } + } + for (const image of images) { + if (!queued.has(image.id)) { + queue.push(image) + queued.add(image.id) + } + } + + const take = (image: JiraImageAttachment | undefined): string | null => { + if (!image) { + return null + } + const index = queue.findIndex((entry) => entry.id === image.id) + // Why: already-consumed images must not re-emit; fall through to positional pairing. + if (index < 0) { + return null + } + queue.splice(index, 1) + return `![${escapeMarkdownAlt(image.filename)}](${image.dataUrl})` + } + + return (attrs: JiraAdfMediaAttrs): string | null => { + if (attrs.id) { + const cached = resolvedByMediaId.get(attrs.id) + if (cached) { + if (stats && !cached.startsWith('*[') && cached.includes('data:')) { + stats.attachmentResolvedCount += 1 + } + return cached + } + } + const alt = attrs.alt?.trim() || 'Image' + if (attrs.url) { + // Why: return placeholder (not null) so non-http / hostile externals do not + // fall through to positional attachment pairing. + if (!/^https?:\/\//i.test(attrs.url)) { + return unresolvedMediaPlaceholder({ ...attrs, alt }) + } + const safeUrl = escapeMarkdownLinkDestination(attrs.url) + if (!safeUrl) { + return unresolvedMediaPlaceholder({ ...attrs, alt }) + } + // External success does not count toward attachment needCount. + return `![${escapeMarkdownAlt(alt)}](${safeUrl})` + } + + let resolved: string | null = null + // Why: ADF media IDs are Media Service UUIDs, not attachment IDs — skip byId + // lookup on attrs.id against attachment map (they never match). + if (attrs.alt?.trim()) { + const matches = byFilename.get(attrs.alt.trim().toLowerCase()) + if (matches && matches.length > 0) { + const stillQueued = matches.find((image) => queue.some((entry) => entry.id === image.id)) + // Why: only take still-queued matches; never re-emit matches[0] after consume. + if (stillQueued) { + resolved = take(stillQueued) + } + } + } + + // Why: take() removes from queue; do not shift first or membership check fails. + if (!resolved && queue.length > 0) { + resolved = take(queue[0]) + } + if (resolved && attrs.id) { + resolvedByMediaId.set(attrs.id, resolved) + } + if (resolved && stats) { + stats.attachmentResolvedCount += 1 + } + return resolved + } +} diff --git a/src/main/jira/attachment-meta.ts b/src/main/jira/attachment-meta.ts new file mode 100644 index 000000000..e7c67e7a2 --- /dev/null +++ b/src/main/jira/attachment-meta.ts @@ -0,0 +1,56 @@ +// Why: image inlined as data URLs over IPC — keep per-image and selection caps modest. +export const MAX_IMAGE_BYTES = 2 * 1024 * 1024 +export const MAX_TOTAL_IMAGE_BYTES = 5 * 1024 * 1024 +export const MAX_IMAGES = 12 + +export type AttachmentMeta = { + id: string + filename: string + mimeType: string + size: number + contentUrl?: string +} + +function asRecord(value: unknown): Record { + return value && typeof value === 'object' ? (value as Record) : {} +} + +function asString(value: unknown): string { + return typeof value === 'string' ? value : '' +} + +export function isImageMimeType(mimeType: string): boolean { + const normalized = mimeType.toLowerCase() + return ( + normalized.startsWith('image/') && !normalized.includes('svg') // Why: SVG can carry script; stick to raster screenshots. + ) +} + +export function parseImageAttachmentMetas(attachmentField: unknown): AttachmentMeta[] { + if (!Array.isArray(attachmentField)) { + return [] + } + const metas: AttachmentMeta[] = [] + for (const item of attachmentField) { + const record = asRecord(item) + const id = asString(record.id) || (typeof record.id === 'number' ? String(record.id) : '') + const filename = asString(record.filename) || `attachment-${id}` + const mimeType = asString(record.mimeType) + const size = typeof record.size === 'number' && Number.isFinite(record.size) ? record.size : 0 + if (!id || !isImageMimeType(mimeType)) { + continue + } + if (size > MAX_IMAGE_BYTES) { + continue + } + const contentUrl = asString(record.content) + metas.push({ + id, + filename, + mimeType, + size, + ...(contentUrl ? { contentUrl } : {}) + }) + } + return metas +} diff --git a/src/main/jira/client.test.ts b/src/main/jira/client.test.ts index 0c0ee1151..e7c3a6963 100644 --- a/src/main/jira/client.test.ts +++ b/src/main/jira/client.test.ts @@ -201,6 +201,40 @@ describe('Jira client credential storage', () => { expect(userAgent).not.toMatch(/Mozilla|Chrome|Safari|AppleWebKit/i) }) + it('downloads same-origin attachment URLs without forwarding auth cross-origin', async () => { + const jira = await loadClientModule({ encryptionAvailable: true }) + const client = { + site: { + id: 'site-alpha', + siteUrl: 'https://example.atlassian.net', + email: 'ada@example.com', + displayName: 'Ada', + accountId: 'account-alpha' + }, + authorization: 'Basic token-alpha' + } + netFetchMock.mockResolvedValueOnce( + new Response(Uint8Array.from([1, 2, 3]), { + status: 200, + headers: { 'Content-Type': 'image/png' } + }) + ) + + await expect( + jira.jiraRequestBinary( + client, + 'https://example.atlassian.net/rest/api/3/attachment/content/1?redirect=false' + ) + ).resolves.toMatchObject({ contentType: 'image/png' }) + const headers = netFetchMock.mock.calls[0]?.[1]?.headers as Headers + expect(headers.get('Authorization')).toBe('Basic token-alpha') + + await expect( + jira.jiraRequestBinary(client, 'https://files.example.com/attachment.png') + ).rejects.toThrow('configured site origin') + expect(netFetchMock).toHaveBeenCalledTimes(1) + }) + it('does not pass encrypted safeStorage bytes to Jira when encryption is unavailable', async () => { const siteId = 'site-alpha' const tokenPath = tokenPathForSite(siteId) diff --git a/src/main/jira/client.ts b/src/main/jira/client.ts index 4c182ee2a..336426ba9 100644 --- a/src/main/jira/client.ts +++ b/src/main/jira/client.ts @@ -21,6 +21,7 @@ import type { JiraSiteSelection, JiraViewer } from '../../shared/types' +import { clearAttachmentImagesForSite } from './attachment-image-cache' // Why: Atlassian's XSRF filter rejects POST/PUT REST calls that carry a browser // User-Agent, failing them with "XSRF check failed" even under API-token auth. @@ -457,6 +458,36 @@ export async function jiraRequest( return (await response.json()) as T } +export async function jiraRequestBinary( + client: JiraClientForSite, + pathOrUrl: string +): Promise<{ data: ArrayBuffer; contentType: string }> { + const siteUrl = new URL(client.site.siteUrl) + const requestUrl = /^https?:\/\//i.test(pathOrUrl) + ? new URL(pathOrUrl) + : new URL(`${client.site.siteUrl}${pathOrUrl}`) + if (requestUrl.origin !== siteUrl.origin) { + // Why: attachment metadata is provider-controlled; never forward Jira + // credentials if a malformed response points at another origin. + throw new JiraApiError('Jira attachment URL must use the configured site origin.', null) + } + const headers = new Headers() + // Why: attachment content is binary; forcing JSON Accept/Content-Type can + // break downloads and confuses some Atlassian edge responses. + headers.set('Accept', '*/*') + headers.set('User-Agent', JIRA_API_USER_AGENT) + headers.set('Authorization', client.authorization) + const response = await jiraFetch(requestUrl.toString(), { headers }) + if (!response.ok) { + throw new JiraApiError(await readJiraError(response), response.status) + } + const contentType = response.headers.get('content-type') || 'application/octet-stream' + return { + data: await response.arrayBuffer(), + contentType + } +} + export function getClients(selection?: JiraSiteSelection | null): JiraClientForSite[] { const file = getSiteFile() const selected = selection ?? file.selectedSiteId ?? file.activeSiteId @@ -576,6 +607,9 @@ export function disconnect(siteId?: string): void { for (const id of ids) { deleteToken(id) } + // Why: drop cached attachment data URLs for disconnected sites so main does + // not retain multi-MB strings after logout. + clearAttachmentImagesForSite(siteId) writeSiteFile({ version: 1, activeSiteId: file.activeSiteId, @@ -625,6 +659,8 @@ export async function testConnection( export function clearToken(siteId: string): void { deleteToken(siteId) + // Why: auth failure removes the site; drop cached attachment data URLs too. + clearAttachmentImagesForSite(siteId) const file = getSiteFile() writeSiteFile({ ...file, sites: file.sites.filter((site) => site.id !== siteId) }) } diff --git a/src/main/jira/issues.test.ts b/src/main/jira/issues.test.ts index 4b20b0c4a..134b7543d 100644 --- a/src/main/jira/issues.test.ts +++ b/src/main/jira/issues.test.ts @@ -2,22 +2,41 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { JiraClientForSite } from './client' import { credentialDecryptionMessage } from '../../shared/integration-credential-errors' -const { clearTokenMock, getClientsMock, isAuthErrorMock, jiraRequestMock } = vi.hoisted(() => ({ +const { + clearTokenMock, + getClientsMock, + isAuthErrorMock, + jiraRequestMock, + jiraRequestBinaryMock, + acquireMock, + releaseMock +} = vi.hoisted(() => ({ clearTokenMock: vi.fn(), getClientsMock: vi.fn(), isAuthErrorMock: vi.fn(), - jiraRequestMock: vi.fn() + jiraRequestMock: vi.fn(), + jiraRequestBinaryMock: vi.fn(), + acquireMock: vi.fn().mockResolvedValue(undefined), + releaseMock: vi.fn() })) vi.mock('./client', () => ({ - acquire: vi.fn().mockResolvedValue(undefined), - release: vi.fn(), + acquire: (...args: unknown[]) => acquireMock(...args), + release: (...args: unknown[]) => releaseMock(...args), 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) + 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 + } + } })) function makeEntry(id = 'site-1'): JiraClientForSite { @@ -48,10 +67,16 @@ function makeServerEntry(id = 'server-1'): JiraClientForSite { } describe('Jira issue operations', () => { - beforeEach(() => { + 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('surfaces Jira credential decrypt errors on active issue, metadata, and mutation paths', async () => { @@ -341,6 +366,73 @@ describe('Jira issue operations', () => { }) }) + it('embeds resolved attachment images into getIssue descriptions', async () => { + const pngBytes = Uint8Array.from([137, 80, 78, 71]) + jiraRequestBinaryMock.mockResolvedValue({ + data: pngBytes.buffer, + contentType: 'image/png' + }) + jiraRequestMock.mockResolvedValue({ + id: 'issue-9', + key: 'CAM-9', + fields: { + summary: 'UI with screenshot', + description: { + type: 'doc', + version: 1, + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'See image' }] }, + { + type: 'mediaSingle', + content: [ + { + type: 'media', + attrs: { id: 'media-uuid', type: 'file', alt: 'ui.png' } + } + ] + } + ] + }, + attachment: [ + { + id: '10001', + filename: 'ui.png', + mimeType: 'image/png', + size: 4 + } + ], + project: { id: '1', key: 'CAM', name: 'CAM' }, + issuetype: { id: '1', name: 'Story' }, + 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' + }, + renderedFields: { + description: + '

See image

' + } + }) + + const { getIssue } = await import('./issues') + const issue = await getIssue('CAM-9', 'site-1') + + expect(jiraRequestMock).toHaveBeenCalledWith( + expect.anything(), + expect.stringContaining('expand=renderedFields') + ) + expect(jiraRequestBinaryMock).toHaveBeenCalledWith( + expect.anything(), + 'https://example.atlassian.net/rest/api/3/attachment/content/10001?redirect=false' + ) + expect(issue?.description).toContain('See image') + expect(issue?.description).toContain('![ui.png](data:image/png;base64,') + }) + it('maps Jira ADF descriptions into Markdown blocks and lists', async () => { const { mapJiraIssue } = await import('./issues') @@ -474,6 +566,161 @@ describe('Jira issue operations', () => { updatedAt: undefined } ]) + expect(jiraRequestMock).toHaveBeenCalledTimes(1) + expect(String(jiraRequestMock.mock.calls[0]?.[1])).toContain('expand=renderedBody') + expect(jiraRequestBinaryMock).not.toHaveBeenCalled() + }) + + it('releases the Jira slot before downloading issue attachment binaries', async () => { + const order: string[] = [] + acquireMock.mockImplementation(async () => { + order.push('acquire') + }) + releaseMock.mockImplementation(() => { + order.push('release') + }) + jiraRequestMock.mockImplementation(async () => { + order.push('json') + return { + id: 'issue-9', + key: 'CAM-9', + fields: { + summary: 'UI with screenshot', + description: { + type: 'doc', + version: 1, + content: [ + { + type: 'mediaSingle', + content: [ + { type: 'media', attrs: { id: 'media-uuid', type: 'file', alt: 'ui.png' } } + ] + } + ] + }, + attachment: [{ id: '10001', filename: 'ui.png', mimeType: 'image/png', size: 4 }], + project: { id: '1', key: 'CAM', name: 'CAM' }, + issuetype: { id: '1', name: 'Bug' }, + status: { id: '1', name: 'To Do', statusCategory: { key: 'new' } }, + labels: [], + created: '2026-05-01T00:00:00.000Z', + updated: '2026-05-01T00:00:00.000Z' + }, + renderedFields: { + description: + '' + } + } + }) + jiraRequestBinaryMock.mockImplementation(async () => { + order.push('binary') + return { data: Uint8Array.from([1]).buffer, contentType: 'image/png' } + }) + const { getIssue } = await import('./issues') + await getIssue('CAM-9', 'site-1') + expect(order.indexOf('release')).toBeLessThan(order.indexOf('binary')) + expect(order.indexOf('json')).toBeLessThan(order.indexOf('release')) + }) + + it('uses Server/DC api base path for comment attachment metadata lookup', async () => { + getClientsMock.mockReturnValue([makeServerEntry()]) + jiraRequestMock + .mockResolvedValueOnce({ + comments: [ + { + id: 'c1', + body: { + type: 'doc', + version: 1, + content: [ + { + type: 'mediaSingle', + content: [{ type: 'media', attrs: { id: 'm1', type: 'file', alt: 'shot.png' } }] + } + ] + }, + renderedBody: '', + created: '2026-05-30T12:00:00.000Z', + author: { accountId: 'u1', displayName: 'Ada' } + } + ] + }) + .mockResolvedValueOnce({ + fields: { + attachment: [ + { + id: '9', + filename: 'shot.png', + mimeType: 'image/png', + size: 4, + content: 'https://jira.example.com/secure/attachment/9/shot.png' + } + ] + } + }) + jiraRequestBinaryMock.mockResolvedValue({ + data: Uint8Array.from([1]).buffer, + contentType: 'image/png' + }) + const { getIssueComments } = await import('./issues') + await getIssueComments('ALP-1', 'server-1') + const attachmentLookup = jiraRequestMock.mock.calls.find((call) => + String(call[1]).includes('fields=attachment') + ) + expect(String(attachmentLookup?.[1])).toContain('/rest/api/2/issue/') + expect(String(attachmentLookup?.[1])).not.toContain('/rest/api/3/issue/') + }) + + it('embeds only attachments referenced by rendered Jira comments', async () => { + jiraRequestMock + .mockResolvedValueOnce({ + comments: [ + { + id: 'comment-1', + body: { + type: 'doc', + version: 1, + content: [ + { + type: 'mediaSingle', + content: [ + { + type: 'media', + attrs: { id: 'media-uuid', type: 'file', alt: 'comment.png' } + } + ] + } + ] + }, + renderedBody: + '', + created: '2026-05-30T12:00:00.000Z', + author: { accountId: 'user-1', displayName: 'Ada' } + } + ] + }) + .mockResolvedValueOnce({ + fields: { + attachment: [ + { id: '20001', filename: 'unrelated.png', mimeType: 'image/png', size: 4 }, + { id: '20002', filename: 'comment.png', mimeType: 'image/png', size: 4 } + ] + } + }) + jiraRequestBinaryMock.mockResolvedValue({ + data: Uint8Array.from([137, 80, 78, 71]).buffer, + contentType: 'image/png' + }) + const { getIssueComments } = await import('./issues') + + const comments = await getIssueComments('ALP-1', 'site-1') + + expect(comments[0]?.body).toContain('![comment.png](data:image/png;base64,') + expect(jiraRequestBinaryMock).toHaveBeenCalledTimes(1) + expect(jiraRequestBinaryMock).toHaveBeenCalledWith( + expect.anything(), + 'https://example.atlassian.net/rest/api/3/attachment/content/20002?redirect=false' + ) }) describe('getProjectStatusOrder', () => { diff --git a/src/main/jira/issues.ts b/src/main/jira/issues.ts index 2ce24a874..339e2276d 100644 --- a/src/main/jira/issues.ts +++ b/src/main/jira/issues.ts @@ -31,7 +31,23 @@ import { release, type JiraClientForSite } from './client' -import { adfToMarkdownText, textToAdf } from './adf-markdown' +import { + adfToMarkdownText, + collectAdfMediaAttrs, + textToAdf, + type AdfToMarkdownOptions, + type JiraAdfMediaAttrs +} from './adf-markdown' +import { + extractAttachmentContentIdsFromHtml, + selectPreferredAttachmentIds, + warnIfMediaResolutionIncomplete +} from './attachment-discovery' +import { + createMediaMarkdownResolver, + loadIssueImageAttachments, + type MediaResolutionStats +} from './attachment-images' const ISSUE_FIELDS = [ 'summary', @@ -47,6 +63,10 @@ const ISSUE_FIELDS = [ 'updated' ] +// Why: detail reads need attachment metadata so inline ADF media can be resolved +// to downloadable image content; list/search omit this for payload size. +const ISSUE_DETAIL_FIELDS = [...ISSUE_FIELDS, 'attachment'] + type JiraRecord = Record type JiraSearchResponse = { @@ -314,7 +334,11 @@ function toBodyText(site: JiraSite, text: string): unknown { return site.authType === 'server' ? text : textToAdf(text) } -export function mapJiraIssue(site: JiraSite, raw: JiraRecord): JiraIssue { +export function mapJiraIssue( + site: JiraSite, + raw: JiraRecord, + adfOptions?: AdfToMarkdownOptions +): JiraIssue { const fields = asRecord(raw.fields) const key = asString(raw.key) return { @@ -323,7 +347,7 @@ export function mapJiraIssue(site: JiraSite, raw: JiraRecord): JiraIssue { siteId: site.id, siteName: site.displayName, title: asString(fields.summary, key || 'Untitled issue'), - description: adfToMarkdownText(fields.description), + description: adfToMarkdownText(fields.description, adfOptions), url: issueUrl(site, key), project: mapProject(fields.project, site), issueType: mapIssueType(fields.issuetype), @@ -337,6 +361,97 @@ export function mapJiraIssue(site: JiraSite, raw: JiraRecord): JiraIssue { } } +type MediaRequest = { + attachmentField: unknown + preferredIds: string[] + needCount: number + fallbackRan: boolean + issueKey: string +} + +/** Pooled: HTML/ADF selection only — no binary downloads. */ +function collectIssueMediaRequest(raw: JiraRecord): MediaRequest | undefined { + const fields = asRecord(raw.fields) + const renderedFields = asRecord(raw.renderedFields) + const htmlIds = extractAttachmentContentIdsFromHtml( + asString(renderedFields.description) || undefined + ) + const mediaAttrs = collectAdfMediaAttrs(fields.description) + const selection = selectPreferredAttachmentIds({ + renderedHtmlIds: htmlIds, + attachmentField: fields.attachment, + mediaAttrs + }) + if (selection.needCount === 0 && selection.preferredIds.length === 0) { + return undefined + } + return { + attachmentField: fields.attachment, + preferredIds: selection.preferredIds, + needCount: selection.needCount, + fallbackRan: selection.fallbackRan, + issueKey: asString(raw.key) + } +} + +type PreparedMedia = { + options: AdfToMarkdownOptions + stats: MediaResolutionStats + request: MediaRequest +} + +/** Unpooled: binary downloads + resolver (outside the Jira API semaphore). */ +async function prepareMediaResolver( + client: JiraClientForSite, + request: MediaRequest +): Promise { + if (request.preferredIds.length === 0) { + warnIfMediaResolutionIncomplete({ + siteId: client.site.id, + issueKey: request.issueKey, + needCount: request.needCount, + preferredIdCount: 0, + resolvedCount: 0, + fallbackRan: request.fallbackRan + }) + return undefined + } + const images = await loadIssueImageAttachments( + client, + request.attachmentField, + request.preferredIds + ) + if (images.length === 0) { + warnIfMediaResolutionIncomplete({ + siteId: client.site.id, + issueKey: request.issueKey, + needCount: request.needCount, + preferredIdCount: request.preferredIds.length, + resolvedCount: 0, + fallbackRan: request.fallbackRan + }) + return undefined + } + const stats: MediaResolutionStats = { attachmentResolvedCount: 0 } + const resolveMedia = createMediaMarkdownResolver(images, request.preferredIds, stats) + return { + options: { resolveMedia }, + stats, + request + } +} + +function flushMediaResolutionWarn(client: JiraClientForSite, prepared: PreparedMedia): void { + warnIfMediaResolutionIncomplete({ + siteId: client.site.id, + issueKey: prepared.request.issueKey, + needCount: prepared.request.needCount, + preferredIdCount: prepared.request.preferredIds.length, + resolvedCount: prepared.stats.attachmentResolvedCount, + fallbackRan: prepared.request.fallbackRan + }) +} + function sortAndLimitIssues(issues: JiraIssue[], limit: number): JiraIssue[] { return issues .sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime()) @@ -437,15 +552,22 @@ export async function getIssue( ): Promise { const entries = getClients(siteId) for (const entry of entries) { - await acquire() + let mediaRequest: MediaRequest | undefined + let issue: JiraRecord | undefined + let held = false try { - const issue = await jiraRequest( + await acquire() + held = true + const params = new URLSearchParams({ + fields: ISSUE_DETAIL_FIELDS.join(','), + expand: 'renderedFields' + }) + issue = await jiraRequest( entry, - `${apiBasePath(entry.site)}/issue/${encodeURIComponent(key)}?fields=${encodeURIComponent( - ISSUE_FIELDS.join(',') - )}` + `${apiBasePath(entry.site)}/issue/${encodeURIComponent(key)}?${params.toString()}` ) - return mapJiraIssue(entry.site, issue) + // Why: keep only JSON under the pool; binary downloads fan out after release. + mediaRequest = collectIssueMediaRequest(issue) } catch (error) { if (isAuthError(error)) { clearToken(entry.site.id) @@ -455,8 +577,27 @@ export async function getIssue( } else { console.warn('[jira] getIssue failed:', error) } + continue } finally { - release() + if (held) { + held = false + release() + } + } + + try { + if (!issue) { + continue + } + const prepared = mediaRequest ? await prepareMediaResolver(entry, mediaRequest) : undefined + const mapped = mapJiraIssue(entry.site, issue, prepared?.options) + if (prepared) { + flushMediaResolutionWarn(entry, prepared) + } + return mapped + } catch (error) { + console.warn('[jira] getIssue media load failed:', error) + return mapJiraIssue(entry.site, issue) } } return null @@ -597,16 +738,79 @@ export async function addIssueComment( } } -function mapComment(raw: JiraRecord): JiraComment { +function mapComment(raw: JiraRecord, adfOptions?: AdfToMarkdownOptions): JiraComment { return { id: asString(raw.id), - body: adfToMarkdownText(raw.body), + body: adfToMarkdownText(raw.body, adfOptions), createdAt: asString(raw.created, new Date().toISOString()), updatedAt: asString(raw.updated) || undefined, user: mapUser(raw.author) } } +/** + * Pooled comment media collect: attachment metadata JSON stays under the semaphore. + * Residual: Server/DC comment bodies are wiki markup, not ADF — this only fixes + * the lookup path; wiki `!filename!` is not rendered as media. + */ +async function collectCommentMediaRequest( + client: JiraClientForSite, + key: string, + comments: JiraRecord[] +): Promise { + const htmlIds: string[] = [] + const seen = new Set() + const mediaAttrs: JiraAdfMediaAttrs[] = [] + for (const comment of comments) { + for (const id of extractAttachmentContentIdsFromHtml(asString(comment.renderedBody))) { + if (!seen.has(id)) { + seen.add(id) + htmlIds.push(id) + } + } + mediaAttrs.push(...collectAdfMediaAttrs(comment.body)) + } + + const needingCount = mediaAttrs.filter( + (attrs) => !(attrs.url && /^https?:\/\//i.test(attrs.url)) + ).length + // Why: selectPreferredAttachmentIds yields nothing without attachment-needing media, so + // HTML ids alone can never produce a download — skip the extra metadata request entirely. + if (needingCount === 0) { + return undefined + } + + // Why: comment media usually references issue-level attachments; pull them once + // for the whole thread. Use apiBasePath so Server/DC does not 404 on /rest/api/3. + let attachmentField: unknown + try { + const issue = await jiraRequest( + client, + `${apiBasePath(client.site)}/issue/${encodeURIComponent(key)}?fields=attachment` + ) + attachmentField = asRecord(issue.fields).attachment + } catch (error) { + console.warn('[jira] comment attachment lookup failed:', error) + return undefined + } + + const selection = selectPreferredAttachmentIds({ + renderedHtmlIds: htmlIds, + attachmentField, + mediaAttrs + }) + if (selection.needCount === 0 && selection.preferredIds.length === 0) { + return undefined + } + return { + attachmentField, + preferredIds: selection.preferredIds, + needCount: selection.needCount, + fallbackRan: selection.fallbackRan, + issueKey: key + } +} + export async function getIssueComments( key: string, siteId?: string | null @@ -615,17 +819,23 @@ export async function getIssueComments( if (!entry) { return [] } - await acquire() + + let comments: JiraRecord[] = [] + let mediaRequest: MediaRequest | undefined + let held = false try { - const comments = await fetchPagedRecords(entry, 'comments', (startAt, maxResults) => { + await acquire() + held = true + comments = await fetchPagedRecords(entry, 'comments', (startAt, maxResults) => { const params = new URLSearchParams({ maxResults: String(maxResults), orderBy: 'created', - startAt: String(startAt) + startAt: String(startAt), + expand: 'renderedBody' }) return `${apiBasePath(entry.site)}/issue/${encodeURIComponent(key)}/comment?${params.toString()}` }) - return comments.map(mapComment) + mediaRequest = await collectCommentMediaRequest(entry, key, comments) } catch (error) { if (isAuthError(error)) { clearToken(entry.site.id) @@ -634,7 +844,22 @@ export async function getIssueComments( console.warn('[jira] getIssueComments failed:', error) return [] } finally { - release() + if (held) { + held = false + release() + } + } + + try { + const prepared = mediaRequest ? await prepareMediaResolver(entry, mediaRequest) : undefined + const mapped = comments.map((comment) => mapComment(comment, prepared?.options)) + if (prepared) { + flushMediaResolutionWarn(entry, prepared) + } + return mapped + } catch (error) { + console.warn('[jira] getIssueComments media load failed:', error) + return comments.map((comment) => mapComment(comment)) } } diff --git a/src/main/runtime/rpc/methods/jira.test.ts b/src/main/runtime/rpc/methods/jira.test.ts index 8bf82eab5..ce02c6ec7 100644 --- a/src/main/runtime/rpc/methods/jira.test.ts +++ b/src/main/runtime/rpc/methods/jira.test.ts @@ -117,6 +117,29 @@ describe('jira RPC methods', () => { expect(runtime.jiraIssueComments).toHaveBeenCalledWith('ABC-3', 'site-1') }) + it('streams Jira image-bearing payloads in bounded JSON chunks', async () => { + const description = `![shot](data:image/png;base64,${'a'.repeat(300_000)})` + const runtime = { + getRuntimeId: () => 'test-runtime', + jiraGetIssue: vi.fn().mockResolvedValue({ key: 'ABC-3', description }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: JIRA_METHODS }) + const replies: string[] = [] + + await dispatcher.dispatchStreaming( + makeRequest('jira.getIssueStream', { key: 'ABC-3', siteId: 'site-1' }), + (response) => replies.push(response) + ) + + const messages = replies.map( + (response) => (JSON.parse(response) as { result: { type: string; content?: string } }).result + ) + expect(messages.at(-1)).toEqual({ type: 'end' }) + expect(messages.filter((message) => message.type === 'chunk')).toHaveLength(2) + const payload = messages.map((message) => message.content ?? '').join('') + expect(JSON.parse(payload)).toEqual({ key: 'ABC-3', description }) + }) + it('routes Jira metadata requests to the runtime server', async () => { const runtime = { getRuntimeId: () => 'test-runtime', diff --git a/src/main/runtime/rpc/methods/jira.ts b/src/main/runtime/rpc/methods/jira.ts index a18eb2d02..2b9324d85 100644 --- a/src/main/runtime/rpc/methods/jira.ts +++ b/src/main/runtime/rpc/methods/jira.ts @@ -1,5 +1,9 @@ import { z } from 'zod' -import { defineMethod, type RpcMethod } from '../core' +import { + JIRA_PAYLOAD_CHUNK_CHARS, + JIRA_PAYLOAD_MAX_CHARS +} from '../../../../shared/jira-payload-stream' +import { defineMethod, defineStreamingMethod, type RpcAnyMethod } from '../core' import { OptionalFiniteNumber, OptionalPlainString, @@ -95,7 +99,20 @@ const ProjectStatusOrder = z.object({ siteId: OptionalString }) -export const JIRA_METHODS: RpcMethod[] = [ +function emitJiraPayload(value: unknown, emit: (result: unknown) => void): void { + const payload = JSON.stringify(value) + if (payload.length > JIRA_PAYLOAD_MAX_CHARS) { + throw new Error('Jira payload exceeded the transfer limit.') + } + // Why: remote runtime WebSocket messages are capped at 1 MiB; chunking keeps + // authenticated inline images usable over SSH without raising that safety cap. + for (let offset = 0; offset < payload.length; offset += JIRA_PAYLOAD_CHUNK_CHARS) { + emit({ type: 'chunk', content: payload.slice(offset, offset + JIRA_PAYLOAD_CHUNK_CHARS) }) + } + emit({ type: 'end' }) +} + +export const JIRA_METHODS: RpcAnyMethod[] = [ defineMethod({ name: 'jira.connect', params: Connect, @@ -144,6 +161,13 @@ export const JIRA_METHODS: RpcMethod[] = [ params: IssueKey, handler: async (params, { runtime }) => runtime.jiraGetIssue(params.key.trim(), params.siteId) }), + defineStreamingMethod({ + name: 'jira.getIssueStream', + params: IssueKey, + handler: async (params, { runtime }, emit) => { + emitJiraPayload(await runtime.jiraGetIssue(params.key.trim(), params.siteId), emit) + } + }), defineMethod({ name: 'jira.createIssue', params: CreateIssue, @@ -175,6 +199,13 @@ export const JIRA_METHODS: RpcMethod[] = [ handler: async (params, { runtime }) => runtime.jiraIssueComments(params.key.trim(), params.siteId) }), + defineStreamingMethod({ + name: 'jira.issueCommentsStream', + params: IssueKey, + handler: async (params, { runtime }, emit) => { + emitJiraPayload(await runtime.jiraIssueComments(params.key.trim(), params.siteId), emit) + } + }), defineMethod({ name: 'jira.listProjects', params: SiteSelection, diff --git a/src/renderer/src/components/JiraIssueWorkspace.tsx b/src/renderer/src/components/JiraIssueWorkspace.tsx index 857df73e5..dd1ccb4f2 100644 --- a/src/renderer/src/components/JiraIssueWorkspace.tsx +++ b/src/renderer/src/components/JiraIssueWorkspace.tsx @@ -755,8 +755,11 @@ export default function JiraIssueWorkspace({
+ {/* Why: Jira comment screenshots need the same preview + affordance without changing compact comment typography. */}
diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index b2fc3db0a..d14db9fcb 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -7111,6 +7111,7 @@ export default function TaskPage(): React.JSX.Element { // Why: when a modal is open, let it own Esc dismissal. if ( dialogWorkItem || + selectedJiraIssue || selectedLinearIssue || newIssueOpen || newLinearIssueOpen || @@ -7155,7 +7156,8 @@ export default function TaskPage(): React.JSX.Element { newIssueOpen, newLinearIssueOpen, newJiraIssueOpen, - selectedLinearIssue + selectedLinearIssue, + selectedJiraIssue ]) useEffect(() => { diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.test.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.test.tsx index b46dd1df6..644469ae2 100644 --- a/src/renderer/src/components/sidebar/CommentMarkdown.test.tsx +++ b/src/renderer/src/components/sidebar/CommentMarkdown.test.tsx @@ -89,6 +89,30 @@ describe('CommentMarkdown', () => { expect(markup).toContain('src="data:image/png;base64,abc123"') }) + it('renders document markdown images with an expand control for the lightbox', () => { + // Why: document bodies need a large preview without a provider-specific renderer. + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain(' { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('aria-label="Expand image"') + expect(markup).toContain('max-h-32') + }) + it('renders bare GitHub user attachment links as document videos', () => { const url = 'https://github.com/user-attachments/assets/ce11040a-fb66-4289-927f-547b16dfc488' const markup = renderToStaticMarkup() diff --git a/src/renderer/src/components/sidebar/CommentMarkdown.tsx b/src/renderer/src/components/sidebar/CommentMarkdown.tsx index 4f28bac6b..8673baede 100644 --- a/src/renderer/src/components/sidebar/CommentMarkdown.tsx +++ b/src/renderer/src/components/sidebar/CommentMarkdown.tsx @@ -185,6 +185,7 @@ type CommentMarkdownProps = React.ComponentPropsWithoutRef<'div'> & { githubRepo?: GitHubRepoReference | null onLinkClick?: CommentMarkdownLinkClickHandler allowFileUriLinks?: boolean + expandImages?: boolean } // Why forwardRef + rest props: Radix's HoverCardTrigger asChild merges a ref @@ -199,6 +200,7 @@ const CommentMarkdown = React.memo( githubRepo, onLinkClick, allowFileUriLinks = false, + expandImages = false, ...rest }, ref @@ -207,12 +209,14 @@ const CommentMarkdown = React.memo( if (!onLinkClick) { return variant === 'document' ? documentCommentMarkdownComponents - : compactCommentMarkdownComponents + : expandImages + ? createCompactCommentMarkdownComponents(undefined, true) + : compactCommentMarkdownComponents } return variant === 'document' ? createDocumentCommentMarkdownComponents(onLinkClick) - : createCompactCommentMarkdownComponents(onLinkClick) - }, [variant, onLinkClick]) + : createCompactCommentMarkdownComponents(onLinkClick, expandImages) + }, [expandImages, variant, onLinkClick]) const activeRemarkPlugins = React.useMemo( () => (githubRepo ? [...remarkPlugins, remarkGitHubReferences(githubRepo)] : remarkPlugins), [githubRepo] diff --git a/src/renderer/src/components/sidebar/MarkdownImageLightbox.test.tsx b/src/renderer/src/components/sidebar/MarkdownImageLightbox.test.tsx new file mode 100644 index 000000000..0570e7fc6 --- /dev/null +++ b/src/renderer/src/components/sidebar/MarkdownImageLightbox.test.tsx @@ -0,0 +1,68 @@ +// @vitest-environment happy-dom +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ExpandableMarkdownImage } from './MarkdownImageLightbox' +import { Sheet, SheetContent, SheetTitle } from '@/components/ui/sheet' + +afterEach(() => { + cleanup() +}) + +describe('ExpandableMarkdownImage', () => { + it('opens an accessible dialog, traps focus, and restores focus after Escape', async () => { + const user = userEvent.setup() + render( + + ) + + const trigger = screen.getByRole('button', { name: 'Expand image' }) + await user.click(trigger) + expect(screen.getByRole('dialog', { name: 'shot.png' })).toBeTruthy() + expect(screen.getAllByAltText('shot.png').length).toBeGreaterThanOrEqual(2) + const closeButton = screen.getByRole('button', { name: 'Close' }) + expect(document.activeElement).toBe(closeButton) + + await user.tab() + expect(document.activeElement).toBe(closeButton) + + await user.keyboard('{Escape}') + await waitFor(() => expect(screen.queryByRole('dialog', { name: 'shot.png' })).toBeNull()) + expect(document.activeElement).toBe(trigger) + }) + + it('closes from the dialog close button', () => { + render() + + fireEvent.click(screen.getByRole('button', { name: 'Expand image' })) + fireEvent.click(screen.getByRole('button', { name: 'Close' })) + expect(screen.queryByRole('dialog', { name: 'ui.png' })).toBeNull() + }) + + it('keeps the parent issue drawer open after Escape and close-button dismissal', () => { + const onOpenChange = vi.fn() + + render( + + + Jira issue + + + + ) + + fireEvent.click(screen.getByRole('button', { name: 'Expand image' })) + fireEvent.keyDown(screen.getByRole('dialog', { name: 'jira.png' }), { key: 'Escape' }) + expect(screen.getByText('Jira issue')).toBeTruthy() + expect(onOpenChange).not.toHaveBeenCalled() + + fireEvent.click(screen.getByRole('button', { name: 'Expand image' })) + fireEvent.click(screen.getByRole('button', { name: 'Close' })) + expect(screen.getByText('Jira issue')).toBeTruthy() + expect(onOpenChange).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/sidebar/MarkdownImageLightbox.tsx b/src/renderer/src/components/sidebar/MarkdownImageLightbox.tsx new file mode 100644 index 000000000..969e9d135 --- /dev/null +++ b/src/renderer/src/components/sidebar/MarkdownImageLightbox.tsx @@ -0,0 +1,82 @@ +import React from 'react' +import { X } from 'lucide-react' +import { + Dialog, + DialogClose, + DialogContent, + DialogTitle, + DialogTrigger +} from '@/components/ui/dialog' +import { Button } from '@/components/ui/button' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' + +type ExpandableMarkdownImageProps = { + src: string + alt?: string + className?: string + triggerClassName?: string +} + +/** + * Inline markdown image that opens a viewport-centered lightbox on click. + * The shared dialog primitive owns modal focus, Escape, and focus restoration. + */ +export function ExpandableMarkdownImage({ + src, + alt, + className, + triggerClassName +}: ExpandableMarkdownImageProps): React.JSX.Element { + const [open, setOpen] = React.useState(false) + const label = + alt?.trim() || translate('auto.components.sidebar.MarkdownImageLightbox.image', 'Image') + + return ( + + + + + + {label} +
+ {label} + + + +
+
+ {label} +
+ + + ) +} diff --git a/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx b/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx index 18cbf9248..18cef393c 100644 --- a/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx +++ b/src/renderer/src/components/sidebar/comment-markdown-element-renderers.tsx @@ -7,6 +7,7 @@ import { isGitHubUserAttachmentUrl, isGitHubUserAttachmentVideoLink } from './comment-markdown-github-attachment-media' +import { ExpandableMarkdownImage } from './MarkdownImageLightbox' export type CommentMarkdownLinkClickHandler = ( event: React.MouseEvent, @@ -50,7 +51,8 @@ function handleMarkdownImageClick( } export function createCompactCommentMarkdownComponents( - onLinkClick?: CommentMarkdownLinkClickHandler + onLinkClick?: CommentMarkdownLinkClickHandler, + expandImages = false ): Components { return { // Strip

wrappers to avoid double margins in the tight card layout. @@ -158,6 +160,17 @@ export function createCompactCommentMarkdownComponents( ) } + if (expandImages) { + return ( + + ) + } + const image = ( {alt} } - const imageClassName = [ - 'my-3 max-h-96 max-w-full rounded-md object-contain', - 'outline outline-1 outline-black/10 dark:outline-white/10', - onLinkClick ? 'cursor-pointer' : '' - ] - .filter(Boolean) - .join(' ') - + if (!src) { + return alt ? {alt} : null + } + // Why: Jira/Linear/GitHub document bodies often embed screenshots; open a + // viewport-centered lightbox so the preview is not trapped in the drawer. + if (onLinkClick) { + const imageClassName = [ + 'my-3 max-h-96 max-w-full rounded-md object-contain', + 'outline outline-1 outline-black/10 dark:outline-white/10', + 'cursor-pointer' + ].join(' ') + return ( + {alt handleMarkdownImageClick(e, src, onLinkClick)} + /> + ) + } return ( - {alt handleMarkdownImageClick(e, src, onLinkClick)} + alt={alt} + className="max-h-96 max-w-full rounded-md object-contain outline outline-1 outline-black/10 dark:outline-white/10" /> ) }, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index a8e950f09..f5cf363fb 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -4919,6 +4919,11 @@ "cancel": "Cancel", "forget": "Remove from Orca", "reconnectAndDelete": "Reconnect & Delete" + }, + "MarkdownImageLightbox": { + "close": "Close", + "image": "Image", + "expand": "Expand image" } }, "shared": { diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 23b52f446..e8158d266 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -4896,6 +4896,11 @@ "cancel": "Cancelar", "forget": "Eliminar de Orca", "reconnectAndDelete": "Reconectar y eliminar" + }, + "MarkdownImageLightbox": { + "close": "Cerrar", + "image": "Imagen", + "expand": "Ampliar imagen" } }, "shared": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 145b4d579..26922f91d 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -4896,6 +4896,11 @@ "cancel": "キャンセル", "forget": "Orcaから削除", "reconnectAndDelete": "再接続して削除" + }, + "MarkdownImageLightbox": { + "close": "閉じる", + "image": "画像", + "expand": "画像を拡大" } }, "shared": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 8522609d0..e80c196ab 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -4896,6 +4896,11 @@ "cancel": "취소", "forget": "Orca에서 제거", "reconnectAndDelete": "재연결 후 삭제" + }, + "MarkdownImageLightbox": { + "close": "닫기", + "image": "이미지", + "expand": "이미지 확대" } }, "shared": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index efc3b530c..b88d6bf99 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -4896,6 +4896,11 @@ "cancel": "取消", "forget": "从 Orca 中移除", "reconnectAndDelete": "重新连接并删除" + }, + "MarkdownImageLightbox": { + "close": "关闭", + "image": "图像", + "expand": "展开图像" } }, "shared": { diff --git a/src/renderer/src/runtime/runtime-jira-client.test.ts b/src/renderer/src/runtime/runtime-jira-client.test.ts index 89fb02ace..2372de76a 100644 --- a/src/renderer/src/runtime/runtime-jira-client.test.ts +++ b/src/renderer/src/runtime/runtime-jira-client.test.ts @@ -1,16 +1,26 @@ // @vitest-environment happy-dom import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { jiraListAssignableUsers, jiraSearchIssues } from './runtime-jira-client' +import { + jiraGetIssue, + jiraIssueComments, + jiraListAssignableUsers, + jiraSearchIssues +} from './runtime-jira-client' + +type RuntimeSubscribeArgs = Parameters[0] +type RuntimeSubscribeCallbacks = Parameters[1] const jiraSearchIssuesLocal = vi.fn() const jiraListAssignableUsersLocal = vi.fn() const runtimeCall = vi.fn() +const runtimeSubscribe = vi.fn() beforeEach(() => { jiraSearchIssuesLocal.mockReset() jiraListAssignableUsersLocal.mockReset() runtimeCall.mockReset() + runtimeSubscribe.mockReset() vi.stubGlobal('window', { api: { jira: { @@ -18,7 +28,8 @@ beforeEach(() => { listAssignableUsers: jiraListAssignableUsersLocal }, runtimeEnvironments: { - call: runtimeCall + call: runtimeCall, + subscribe: runtimeSubscribe } } }) @@ -49,4 +60,57 @@ describe('runtime Jira client search bounds', () => { expect(jiraListAssignableUsersLocal).not.toHaveBeenCalled() expect(runtimeCall).not.toHaveBeenCalled() }) + + it('streams image-bearing issue and comment payloads from remote runtimes', async () => { + runtimeSubscribe.mockImplementation( + async (args: RuntimeSubscribeArgs, callbacks: RuntimeSubscribeCallbacks) => { + const payload = + args.method === 'jira.getIssueStream' + ? { key: 'ORCA-1', description: '![shot](data:image/png;base64,abc)' } + : [{ id: 'comment-1', body: '![shot](data:image/png;base64,abc)' }] + callbacks.onResponse({ + id: 'rpc-1', + ok: true, + result: { type: 'chunk', content: JSON.stringify(payload) }, + _meta: { runtimeId: 'runtime-1' } + }) + callbacks.onResponse({ + id: 'rpc-1', + ok: true, + result: { type: 'end' }, + _meta: { runtimeId: 'runtime-1' } + }) + return { unsubscribe: vi.fn(), sendBinary: vi.fn() } + } + ) + + await expect( + jiraGetIssue({ activeRuntimeEnvironmentId: 'env-1' }, 'ORCA-1', 'site-1') + ).resolves.toMatchObject({ key: 'ORCA-1' }) + await expect( + jiraIssueComments({ activeRuntimeEnvironmentId: 'env-1' }, 'ORCA-1', 'site-1') + ).resolves.toMatchObject([{ id: 'comment-1' }]) + + expect(runtimeSubscribe).toHaveBeenNthCalledWith( + 1, + { + selector: 'env-1', + method: 'jira.getIssueStream', + params: { key: 'ORCA-1', siteId: 'site-1' }, + timeoutMs: 60_000 + }, + expect.anything() + ) + expect(runtimeSubscribe).toHaveBeenNthCalledWith( + 2, + { + selector: 'env-1', + method: 'jira.issueCommentsStream', + params: { key: 'ORCA-1', siteId: 'site-1' }, + timeoutMs: 60_000 + }, + expect.anything() + ) + expect(runtimeCall).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/runtime/runtime-jira-client.ts b/src/renderer/src/runtime/runtime-jira-client.ts index a6c058ac9..a411d47e1 100644 --- a/src/renderer/src/runtime/runtime-jira-client.ts +++ b/src/renderer/src/runtime/runtime-jira-client.ts @@ -19,12 +19,13 @@ import type { JiraUser, JiraViewer } from '../../../shared/types' -import { callRuntimeRpc, getActiveRuntimeTarget } from './runtime-rpc-client' +import { callRuntimeRpc, getActiveRuntimeTarget, RuntimeRpcCallError } from './runtime-rpc-client' import { getTaskSourceRuntimeSettings, type TaskSourceContext } from '../../../shared/task-source-context' import { isRuntimeProviderSearchQueryWithinLimit } from './runtime-provider-search-bounds' +import { readRuntimeJiraPayload } from './runtime-jira-payload-stream' export type RuntimeJiraSettings = | Pick @@ -49,6 +50,24 @@ function getJiraRuntimeTarget( ) } +async function readRemoteJiraPayload( + target: { kind: 'environment'; environmentId: string }, + streamMethod: string, + fallbackMethod: string, + args: unknown +): Promise { + try { + return await readRuntimeJiraPayload(target, streamMethod, args) + } catch (error) { + if (!(error instanceof RuntimeRpcCallError) || error.code !== 'method_not_found') { + throw error + } + // Older runtimes predate image payload streaming but still return text-only + // Jira details safely through the original one-shot method. + return callRuntimeRpc(target, fallbackMethod, args, { timeoutMs: 30_000 }) + } +} + export async function jiraStatus(settings: RuntimeJiraSettings): Promise { const target = getJiraRuntimeTarget(settings) return target.kind === 'environment' @@ -147,7 +166,7 @@ export async function jiraGetIssue( const target = getJiraRuntimeTarget(settings) const args = { key, siteId: siteId ?? undefined } return target.kind === 'environment' - ? callRuntimeRpc(target, 'jira.getIssue', args, { timeoutMs: 30_000 }) + ? readRemoteJiraPayload(target, 'jira.getIssueStream', 'jira.getIssue', args) : window.api.jira.getIssue(args) } @@ -197,7 +216,12 @@ export async function jiraIssueComments( const target = getJiraRuntimeTarget(settings) const args = { key, siteId: siteId ?? undefined } return target.kind === 'environment' - ? callRuntimeRpc(target, 'jira.issueComments', args, { timeoutMs: 30_000 }) + ? readRemoteJiraPayload( + target, + 'jira.issueCommentsStream', + 'jira.issueComments', + args + ) : window.api.jira.issueComments(args) } diff --git a/src/renderer/src/runtime/runtime-jira-payload-stream.test.ts b/src/renderer/src/runtime/runtime-jira-payload-stream.test.ts new file mode 100644 index 000000000..d75d7582b --- /dev/null +++ b/src/renderer/src/runtime/runtime-jira-payload-stream.test.ts @@ -0,0 +1,74 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it, vi } from 'vitest' +import { readRuntimeJiraPayload } from './runtime-jira-payload-stream' + +type RuntimeEnvironmentSubscribeCallbacks = Parameters< + typeof window.api.runtimeEnvironments.subscribe +>[1] + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('readRuntimeJiraPayload', () => { + it('reassembles chunks and unsubscribes when the stream ends', async () => { + const unsubscribe = vi.fn() + const subscribe = vi.fn( + async (_args: unknown, callbacks: RuntimeEnvironmentSubscribeCallbacks) => { + callbacks.onResponse({ + id: 'rpc-1', + ok: true, + result: { type: 'chunk', content: '{"key":' }, + _meta: { runtimeId: 'runtime-1' } + }) + callbacks.onResponse({ + id: 'rpc-1', + ok: true, + result: { type: 'chunk', content: '"ABC-3"}' }, + _meta: { runtimeId: 'runtime-1' } + }) + callbacks.onResponse({ + id: 'rpc-1', + ok: true, + result: { type: 'end' }, + _meta: { runtimeId: 'runtime-1' } + }) + return { unsubscribe, sendBinary: vi.fn() } + } + ) + vi.stubGlobal('window', { api: { runtimeEnvironments: { subscribe } } }) + + await expect( + readRuntimeJiraPayload<{ key: string }>( + { kind: 'environment', environmentId: 'env-1' }, + 'jira.getIssueStream', + { key: 'ABC-3' } + ) + ).resolves.toEqual({ key: 'ABC-3' }) + expect(unsubscribe).toHaveBeenCalledTimes(1) + }) + + it('rejects malformed stream messages', async () => { + const subscribe = vi.fn( + async (_args: unknown, callbacks: RuntimeEnvironmentSubscribeCallbacks) => { + callbacks.onResponse({ + id: 'rpc-1', + ok: true, + result: { type: 'unknown' }, + _meta: { runtimeId: 'runtime-1' } + }) + return { unsubscribe: vi.fn(), sendBinary: vi.fn() } + } + ) + vi.stubGlobal('window', { api: { runtimeEnvironments: { subscribe } } }) + + await expect( + readRuntimeJiraPayload( + { kind: 'environment', environmentId: 'env-1' }, + 'jira.getIssueStream', + { key: 'ABC-3' } + ) + ).rejects.toThrow('invalid message') + }) +}) diff --git a/src/renderer/src/runtime/runtime-jira-payload-stream.ts b/src/renderer/src/runtime/runtime-jira-payload-stream.ts new file mode 100644 index 000000000..1a7f8e194 --- /dev/null +++ b/src/renderer/src/runtime/runtime-jira-payload-stream.ts @@ -0,0 +1,94 @@ +import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' +import { + isJiraPayloadStreamMessage, + JIRA_PAYLOAD_MAX_CHARS +} from '../../../shared/jira-payload-stream' +import { RuntimeRpcCallError } from './runtime-rpc-client' + +type RuntimeJiraPayloadTarget = { kind: 'environment'; environmentId: string } + +export async function readRuntimeJiraPayload( + target: RuntimeJiraPayloadTarget, + method: string, + params: unknown +): Promise { + const chunks: string[] = [] + let receivedChars = 0 + let unsubscribe: (() => void) | null = null + let unsubscribeWhenReady = false + + const close = (): void => { + if (unsubscribe) { + unsubscribe() + } else { + unsubscribeWhenReady = true + } + } + + return new Promise((resolve, reject) => { + let settled = false + const fail = (error: unknown): void => { + if (settled) { + return + } + settled = true + close() + reject(error) + } + const finish = (): void => { + if (settled) { + return + } + settled = true + close() + try { + resolve(JSON.parse(chunks.join('')) as TResult) + } catch { + reject(new Error('Remote Jira payload was not valid JSON.')) + } + } + + void window.api.runtimeEnvironments + .subscribe( + { + selector: target.environmentId, + method, + params, + timeoutMs: 60_000 + }, + { + onResponse: (response) => { + const rpcResponse = response as RuntimeRpcResponse + if (!rpcResponse.ok) { + fail(new RuntimeRpcCallError(rpcResponse)) + return + } + const message = rpcResponse.result + if (!isJiraPayloadStreamMessage(message)) { + fail(new Error('Remote Jira payload stream returned an invalid message.')) + return + } + if (message.type === 'end') { + finish() + return + } + receivedChars += message.content.length + if (receivedChars > JIRA_PAYLOAD_MAX_CHARS) { + fail(new Error('Remote Jira payload exceeded the transfer limit.')) + return + } + chunks.push(message.content) + }, + onError: fail, + onClose: () => fail(new Error('Remote Jira payload stream closed before completion.')) + } + ) + .then((handle) => { + unsubscribe = handle.unsubscribe + if (unsubscribeWhenReady) { + unsubscribe() + } + }) + .catch(fail) + }) +} diff --git a/src/shared/jira-payload-stream.ts b/src/shared/jira-payload-stream.ts new file mode 100644 index 000000000..6f5a76efe --- /dev/null +++ b/src/shared/jira-payload-stream.ts @@ -0,0 +1,12 @@ +export const JIRA_PAYLOAD_CHUNK_CHARS = 256 * 1024 +export const JIRA_PAYLOAD_MAX_CHARS = 32 * 1024 * 1024 + +export type JiraPayloadStreamMessage = { type: 'chunk'; content: string } | { type: 'end' } + +export function isJiraPayloadStreamMessage(value: unknown): value is JiraPayloadStreamMessage { + if (!value || typeof value !== 'object' || !('type' in value)) { + return false + } + const message = value as { type?: unknown; content?: unknown } + return message.type === 'end' || (message.type === 'chunk' && typeof message.content === 'string') +}