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') +}