From 08470f8b8e3f18e3ee4e471e7134a3f8ca845d88 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 27 Jul 2026 12:58:12 -0700 Subject: [PATCH] fix(markdown): bound Markdown document listing and renderer retention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Markdown link completion listed every .md/.mdx/.markdown file under a workspace root with no ceiling on count, aggregate metadata, path length, directories visited, or recursion depth, and the renderer retained one document array per worktree forever. A large monorepo — or a symlink-free but very deep tree — could grow both the main-process listing and the renderer cache without limit. - Add a shared listing budget (20,000 documents / 8 MiB metadata / 64 KiB path / 100,000 visited entries / depth 256) enforced by the local, SSH, and relay producers alike. Caller-supplied limits are clamped down, never up, so the ceiling is enforced rather than advisory. - Stream directory entries with opendir instead of materializing each directory's full entry array. - Bound renderer retention to an LRU of 8 worktree snapshots capped at 32 MiB aggregate. - Surface capacity failures as a typed error (relay code -33008) and a single toast rather than a silent empty list. --- .../pr-sidebar/markdown-blocks.test.ts | 38 ++- .../components/pr-sidebar/markdown-blocks.ts | 80 +++++- ...ile-session-markdown-doc-lifecycle.test.ts | 106 ++++++++ .../mobile-session-markdown-doc-lifecycle.ts | 147 +++++++++++ src/main/ipc/filesystem.test.ts | 4 + src/main/ipc/markdown-documents.test.ts | 55 +++- src/main/ipc/markdown-documents.ts | 83 +++--- .../filesystem-path-listing-provider.ts | 1 + src/main/providers/ssh-filesystem-provider.ts | 3 + .../ssh-markdown-document-listing.test.ts | 42 ++++ .../ssh-markdown-document-listing.ts | 27 ++ src/main/ssh/relay-protocol.ts | 3 +- src/relay/markdown-document-listing.test.ts | 93 +++++++ src/relay/markdown-document-listing.ts | 238 ++++++++++++++++++ src/relay/protocol.ts | 3 +- .../src/components/editor/MonacoEditor.tsx | 10 +- .../editor/markdown-doc-links.test.ts | 4 + .../components/editor/markdown-doc-links.ts | 10 +- .../markdown-document-list-request.test.ts | 27 ++ .../editor/markdown-document-list-request.ts | 17 +- ...rkdown-document-worktree-retention.test.ts | 104 ++++++++ .../markdown-document-worktree-retention.ts | 41 +++ .../editor/markdown-export-extract.test.ts | 37 ++- .../editor/markdown-export-extract.ts | 87 +++++-- .../monaco-markdown-doc-completions.test.ts | 68 +++++ .../editor/monaco-markdown-doc-completions.ts | 109 +++++++- .../editor/rich-markdown-local-image.test.ts | 2 +- .../editor/useMarkdownDocuments.test.tsx | 96 +++++++ .../components/editor/useMarkdownDocuments.ts | 34 ++- .../markdown-document-listing-limits.test.ts | 63 +++++ 30 files changed, 1540 insertions(+), 92 deletions(-) create mode 100644 mobile/src/session/mobile-session-markdown-doc-lifecycle.test.ts create mode 100644 mobile/src/session/mobile-session-markdown-doc-lifecycle.ts create mode 100644 src/main/providers/ssh-markdown-document-listing.test.ts create mode 100644 src/main/providers/ssh-markdown-document-listing.ts create mode 100644 src/relay/markdown-document-listing.test.ts create mode 100644 src/relay/markdown-document-listing.ts create mode 100644 src/renderer/src/components/editor/markdown-document-worktree-retention.test.ts create mode 100644 src/renderer/src/components/editor/markdown-document-worktree-retention.ts create mode 100644 src/renderer/src/components/editor/monaco-markdown-doc-completions.test.ts create mode 100644 src/renderer/src/components/editor/useMarkdownDocuments.test.tsx diff --git a/mobile/src/components/pr-sidebar/markdown-blocks.test.ts b/mobile/src/components/pr-sidebar/markdown-blocks.test.ts index ab0026854b4..c140e39ddf8 100644 --- a/mobile/src/components/pr-sidebar/markdown-blocks.test.ts +++ b/mobile/src/components/pr-sidebar/markdown-blocks.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { parseInline, parseMarkdownBlocks } from './markdown-blocks' +import { COMMENT_MARKDOWN_LIMITS, parseInline, parseMarkdownBlocks } from './markdown-blocks' describe('parseMarkdownBlocks', () => { it('classifies headings, fenced code, quotes, lists, hr, and paragraphs', () => { @@ -100,6 +100,42 @@ describe('parseMarkdownBlocks tables', () => { ]) }) + it('accepts the exact table column limit and falls back one column later', () => { + const header = Array.from({ length: COMMENT_MARKDOWN_LIMITS.tableColumns }, (_, index) => + String(index) + ).join('|') + const delimiter = Array.from( + { length: COMMENT_MARKDOWN_LIMITS.tableColumns }, + () => '---' + ).join('|') + + expect(parseMarkdownBlocks(`${header}\n${delimiter}`)[0]).toMatchObject({ + kind: 'table', + headers: { length: COMMENT_MARKDOWN_LIMITS.tableColumns } + }) + expect(parseMarkdownBlocks(`${header}|extra\n${delimiter}|---`)[0]).toMatchObject({ + kind: 'paragraph' + }) + }) + + it('falls back to a bounded preview before splitting oversized input', () => { + const exact = 'x'.repeat(COMMENT_MARKDOWN_LIMITS.sourceCodeUnits) + const content = 'x'.repeat(COMMENT_MARKDOWN_LIMITS.sourceCodeUnits + 1) + const [block] = parseMarkdownBlocks(content) + + expect(parseMarkdownBlocks(exact)).toEqual([{ kind: 'paragraph', text: exact }]) + expect(block).toMatchObject({ kind: 'paragraph' }) + expect(block?.kind === 'paragraph' ? block.text.length : 0).toBeLessThan( + COMMENT_MARKDOWN_LIMITS.fallbackCodeUnits + 100 + ) + }) + + it('checks normalized break tags before allocating line arrays', () => { + const content = '
'.repeat(COMMENT_MARKDOWN_LIMITS.lines + 1) + + expect(parseMarkdownBlocks(content)[0]).toMatchObject({ kind: 'paragraph' }) + }) + it('reads per-column alignment from the delimiter row', () => { const md = ['| L | C | R |', '| :--- | :---: | ---: |', '| a | b | c |'].join('\n') const block = parseMarkdownBlocks(md)[0] diff --git a/mobile/src/components/pr-sidebar/markdown-blocks.ts b/mobile/src/components/pr-sidebar/markdown-blocks.ts index d5866585b68..dd597383914 100644 --- a/mobile/src/components/pr-sidebar/markdown-blocks.ts +++ b/mobile/src/components/pr-sidebar/markdown-blocks.ts @@ -40,6 +40,20 @@ const ORDERED = /^\s*\d+[.)]\s+(.*)$/ const HTML_BLOCK = /<(details|blockquote)\b[^>]*>([\s\S]*?)<\/\1>/i const SUMMARY = /]*>([\s\S]*?)<\/summary>/i +export const COMMENT_MARKDOWN_LIMITS = { + sourceCodeUnits: 1024 * 1024, + lines: 10_000, + detailsDepth: 64, + tableColumns: 256, + tableRows: 2000, + tableCells: 50_000, + fallbackCodeUnits: 64 * 1024 +} as const + +const LIMIT_FALLBACK_SUFFIX = '\n\n[Comment preview truncated for safety]' + +class CommentMarkdownLimitError extends Error {} + // Removes residual HTML tags from rendered text so stray // etc. don't // show literally. Conservative: only matches `` / `` shapes, so a bare // "a < b" in prose is left alone. @@ -48,9 +62,56 @@ export function stripHtmlTags(text: string): string { } export function parseMarkdownBlocks(content: string): MarkdownBlock[] { + if (!isCommentMarkdownWithinPreparseLimits(content)) { + return commentMarkdownLimitFallback(content) + } // Drop HTML comments and normalize
before block parsing. const cleaned = content.replace(//g, '').replace(//gi, '\n') - return parseSegment(cleaned) + if (!isCommentMarkdownWithinPreparseLimits(cleaned)) { + return commentMarkdownLimitFallback(content) + } + try { + return parseSegment(cleaned) + } catch (error) { + if (error instanceof CommentMarkdownLimitError) { + return commentMarkdownLimitFallback(content) + } + throw error + } +} + +function isCommentMarkdownWithinPreparseLimits(content: string): boolean { + if (content.length > COMMENT_MARKDOWN_LIMITS.sourceCodeUnits) { + return false + } + let lines = 1 + let detailsDepth = 0 + const detailsPattern = /<\/?details\b/gi + for (let index = 0; index < content.length; index += 1) { + if (content.charCodeAt(index) !== 10) { + continue + } + lines += 1 + if (lines > COMMENT_MARKDOWN_LIMITS.lines) { + return false + } + } + for (const match of content.matchAll(detailsPattern)) { + if (match[0][1] === '/') { + detailsDepth = Math.max(0, detailsDepth - 1) + continue + } + detailsDepth += 1 + if (detailsDepth > COMMENT_MARKDOWN_LIMITS.detailsDepth) { + return false + } + } + return true +} + +function commentMarkdownLimitFallback(content: string): MarkdownBlock[] { + const preview = content.slice(0, COMMENT_MARKDOWN_LIMITS.fallbackCodeUnits) + return [{ kind: 'paragraph', text: `${preview}${LIMIT_FALLBACK_SUFFIX}` }] } // Splits a segment at top-level
/
regions (preserving order), @@ -120,8 +181,17 @@ function parseLines(content: string): MarkdownBlock[] { const align = parseAlignRow(lines[i + 1]) i += 2 const rows: string[][] = [] + let tableCells = headers.length while (i < lines.length && lines[i].includes('|') && lines[i].trim() !== '') { - rows.push(splitTableRow(lines[i])) + if (rows.length >= COMMENT_MARKDOWN_LIMITS.tableRows) { + throw new CommentMarkdownLimitError('Table row limit exceeded') + } + const row = splitTableRow(lines[i]) + tableCells += row.length + if (tableCells > COMMENT_MARKDOWN_LIMITS.tableCells) { + throw new CommentMarkdownLimitError('Table cell limit exceeded') + } + rows.push(row) i += 1 } blocks.push({ kind: 'table', headers, rows, align }) @@ -207,12 +277,18 @@ function splitTableRow(line: string): string[] { continue } if (ch === '|') { + if (cells.length >= COMMENT_MARKDOWN_LIMITS.tableColumns) { + throw new CommentMarkdownLimitError('Table column limit exceeded') + } cells.push(cell.trim()) cell = '' continue } cell += ch } + if (cells.length >= COMMENT_MARKDOWN_LIMITS.tableColumns) { + throw new CommentMarkdownLimitError('Table column limit exceeded') + } cells.push(cell.trim()) return cells } diff --git a/mobile/src/session/mobile-session-markdown-doc-lifecycle.test.ts b/mobile/src/session/mobile-session-markdown-doc-lifecycle.test.ts new file mode 100644 index 00000000000..ce566029f3e --- /dev/null +++ b/mobile/src/session/mobile-session-markdown-doc-lifecycle.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it } from 'vitest' +import type { + MarkdownDocState, + MobileSessionTab +} from '../../app/h/[hostId]/session/mobile-session-route-types' +import { MobileSessionMarkdownDocLifecycle } from './mobile-session-markdown-doc-lifecycle' + +type MarkdownTab = Extract +type ReadyMarkdownDoc = Extract + +function markdownTab(id: string, relativePath = `${id}.md`): MarkdownTab { + return { + type: 'markdown', + id, + title: relativePath, + filePath: `/repo/${relativePath}`, + relativePath, + isDirty: false, + isActive: false, + documentVersion: 'v1' + } +} + +function readyDoc(content: string, isDirty = false): ReadyMarkdownDoc { + return { + status: 'ready', + content, + localContent: content, + baseVersion: 'v1', + isDirty, + editable: true + } +} + +describe('mobile session markdown document lifecycle', () => { + it('retains payloads only for markdown tabs in the accepted snapshot', () => { + const lifecycle = new MobileSessionMarkdownDocLifecycle() + let docs = new Map([ + ['closed', readyDoc('x'.repeat(1_000_000))], + ['live', readyDoc('live')] + ]) + + lifecycle.reconcile([markdownTab('live')], (update) => { + docs = update(docs) + }) + + expect([...docs.keys()]).toEqual(['live']) + }) + + it('retains a dirty orphan when reconciliation keeps its draft tab', () => { + const lifecycle = new MobileSessionMarkdownDocLifecycle() + let docs = new Map([['draft', readyDoc('unsaved', true)]]) + + lifecycle.reconcile([markdownTab('draft')], (update) => { + docs = update(docs) + }) + + expect(docs.get('draft')).toEqual(readyDoc('unsaved', true)) + }) + + it('does not let a late read resurrect a closed tab', async () => { + const lifecycle = new MobileSessionMarkdownDocLifecycle() + const tab = markdownTab('late') + let docs = new Map() + const updateDocs = (update: (current: typeof docs) => typeof docs) => { + docs = update(docs) + } + lifecycle.reconcile([tab], updateDocs) + let resolveRead: (doc: ReadyMarkdownDoc) => void = () => undefined + const read = new Promise((resolve) => { + resolveRead = resolve + }) + + const pending = lifecycle.load(tab, updateDocs, () => read) + lifecycle.close(tab.id, updateDocs) + resolveRead(readyDoc('late payload')) + await pending + + expect(docs.size).toBe(0) + }) + + it('does not let an old path overwrite a replacement using the same tab id', async () => { + const lifecycle = new MobileSessionMarkdownDocLifecycle() + const oldTab = markdownTab('same', 'old.md') + let docs = new Map() + const updateDocs = (update: (current: typeof docs) => typeof docs) => { + docs = update(docs) + } + lifecycle.reconcile([oldTab], updateDocs) + let resolveRead: (doc: ReadyMarkdownDoc) => void = () => undefined + const pending = lifecycle.load( + oldTab, + updateDocs, + () => + new Promise((resolve) => { + resolveRead = resolve + }) + ) + + lifecycle.reconcile([markdownTab('same', 'new.md')], updateDocs) + resolveRead(readyDoc('old payload')) + await pending + + expect(docs.size).toBe(0) + }) +}) diff --git a/mobile/src/session/mobile-session-markdown-doc-lifecycle.ts b/mobile/src/session/mobile-session-markdown-doc-lifecycle.ts new file mode 100644 index 00000000000..29554ce3c69 --- /dev/null +++ b/mobile/src/session/mobile-session-markdown-doc-lifecycle.ts @@ -0,0 +1,147 @@ +import type { + MarkdownDocState, + MobileSessionTab +} from '../../app/h/[hostId]/session/mobile-session-route-types' + +type MobileMarkdownTab = Extract + +type MobileMarkdownDocUpdater = ( + update: (current: Map) => Map +) => void + +type MobileMarkdownDocLoadToken = { + generation: number + requestId: number + tabId: string + tabIdentity: string +} + +export class MobileSessionMarkdownDocLifecycle { + private generation = 0 + private nextRequestId = 0 + private readonly liveTabIdentityById = new Map() + private readonly activeRequestIdByTabId = new Map() + + reconcile(tabs: readonly MobileSessionTab[], updateDocs: MobileMarkdownDocUpdater): void { + const nextIdentityById = new Map( + tabs + .filter((tab): tab is MobileMarkdownTab => tab.type === 'markdown') + .map((tab) => [tab.id, markdownTabIdentity(tab)]) + ) + const replacedTabIds = new Set() + for (const [tabId, currentIdentity] of this.liveTabIdentityById) { + const nextIdentity = nextIdentityById.get(tabId) + if (nextIdentity && nextIdentity !== currentIdentity) { + replacedTabIds.add(tabId) + } + } + for (const tabId of this.activeRequestIdByTabId.keys()) { + if (nextIdentityById.get(tabId) !== this.liveTabIdentityById.get(tabId)) { + this.activeRequestIdByTabId.delete(tabId) + } + } + this.liveTabIdentityById.clear() + for (const [tabId, identity] of nextIdentityById) { + this.liveTabIdentityById.set(tabId, identity) + } + updateDocs((current) => retainLiveMarkdownDocs(current, nextIdentityById, replacedTabIds)) + } + + async load( + tab: MobileMarkdownTab, + updateDocs: MobileMarkdownDocUpdater, + read: () => Promise> + ): Promise { + const token = this.beginLoad(tab) + if (!token) { + return + } + updateDocs((current) => new Map(current).set(tab.id, { status: 'loading' })) + try { + const doc = await read() + if (this.finishLoad(token)) { + updateDocs((current) => new Map(current).set(tab.id, doc)) + } + } catch { + if (this.finishLoad(token)) { + updateDocs((current) => + new Map(current).set(tab.id, { + status: 'error', + message: "Couldn't load markdown" + }) + ) + } + } + } + + close(tabId: string, updateDocs: MobileMarkdownDocUpdater): void { + this.liveTabIdentityById.delete(tabId) + this.activeRequestIdByTabId.delete(tabId) + updateDocs((current) => removeMarkdownDoc(current, tabId)) + } + + reset(): void { + this.generation += 1 + this.liveTabIdentityById.clear() + this.activeRequestIdByTabId.clear() + } + + private beginLoad(tab: MobileMarkdownTab): MobileMarkdownDocLoadToken | null { + const tabIdentity = markdownTabIdentity(tab) + if (this.liveTabIdentityById.get(tab.id) !== tabIdentity) { + return null + } + const requestId = ++this.nextRequestId + this.activeRequestIdByTabId.set(tab.id, requestId) + return { + generation: this.generation, + requestId, + tabId: tab.id, + tabIdentity + } + } + + private finishLoad(token: MobileMarkdownDocLoadToken): boolean { + if ( + token.generation !== this.generation || + this.liveTabIdentityById.get(token.tabId) !== token.tabIdentity || + this.activeRequestIdByTabId.get(token.tabId) !== token.requestId + ) { + return false + } + this.activeRequestIdByTabId.delete(token.tabId) + return true + } +} + +function retainLiveMarkdownDocs( + docs: Map, + liveIdentityById: ReadonlyMap, + replacedTabIds: ReadonlySet +): Map { + let next: Map | null = null + for (const tabId of docs.keys()) { + if (liveIdentityById.has(tabId) && !replacedTabIds.has(tabId)) { + continue + } + next ??= new Map(docs) + next.delete(tabId) + } + return next ?? docs +} + +function removeMarkdownDoc( + docs: Map, + tabId: string +): Map { + if (!docs.has(tabId)) { + return docs + } + const next = new Map(docs) + next.delete(tabId) + return next +} + +function markdownTabIdentity(tab: MobileMarkdownTab): string { + return JSON.stringify([tab.id, tab.filePath, tab.relativePath]) +} diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index b3465ac1617..7e80914189e 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -114,6 +114,10 @@ vi.mock('electron', () => ({ vi.mock('fs/promises', () => ({ readdir: readdirMock, + // Why: markdown discovery streams directory entries via opendir instead of + // materializing them. `for await` accepts a plain array, so delegating keeps + // the readdir fixtures below as the single source of directory contents. + opendir: (dirPath: string) => readdirMock(dirPath, { withFileTypes: true }), readFile: readFileMock, writeFile: writeFileMock, stat: statMock, diff --git a/src/main/ipc/markdown-documents.test.ts b/src/main/ipc/markdown-documents.test.ts index b23f5748cb4..b500acb3c14 100644 --- a/src/main/ipc/markdown-documents.test.ts +++ b/src/main/ipc/markdown-documents.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { markdownDocumentFromFilePath } from './markdown-documents' +import { + markdownDocumentFromFilePath, + markdownDocumentsFromRelativePaths +} from './markdown-documents' +import { MarkdownDocumentListingCapacityError } from '../../shared/markdown-document-listing-limits' describe('markdownDocumentFromFilePath', () => { it('keeps in-root path segments that merely start with parent traversal text', () => { @@ -24,3 +28,52 @@ describe('markdownDocumentFromFilePath', () => { }) }) }) + +describe('markdownDocumentsFromRelativePaths', () => { + it('preserves filtering and sorted output below every limit', () => { + expect( + markdownDocumentsFromRelativePaths('/workspace', [ + 'z-last.markdown', + 'src/app.ts', + '../outside.md', + 'docs/Guide.MDX', + 'README.md' + ]) + ).toEqual([ + { + filePath: '/workspace/docs/Guide.MDX', + relativePath: 'docs/Guide.MDX', + basename: 'Guide.MDX', + name: 'Guide' + }, + { + filePath: '/workspace/README.md', + relativePath: 'README.md', + basename: 'README.md', + name: 'README' + }, + { + filePath: '/workspace/z-last.markdown', + relativePath: 'z-last.markdown', + basename: 'z-last.markdown', + name: 'z-last' + } + ]) + }) + + it('rejects count, metadata, and UTF-8 path overflow before retaining another result', () => { + expect(() => + markdownDocumentsFromRelativePaths('/workspace', ['one.md', 'two.md'], { + maxDocuments: 1 + }) + ).toThrow(MarkdownDocumentListingCapacityError) + expect(() => + markdownDocumentsFromRelativePaths('/workspace', ['metadata.md'], { + maxMetadataBytes: 1 + }) + ).toThrow(MarkdownDocumentListingCapacityError) + expect(() => + markdownDocumentsFromRelativePaths('/workspace', [`${'é'.repeat(40_000)}.md`]) + ).toThrow(MarkdownDocumentListingCapacityError) + }) +}) diff --git a/src/main/ipc/markdown-documents.ts b/src/main/ipc/markdown-documents.ts index 713adc9e0fb..687f50e1c5e 100644 --- a/src/main/ipc/markdown-documents.ts +++ b/src/main/ipc/markdown-documents.ts @@ -1,14 +1,22 @@ -import { readdir } from 'node:fs/promises' -import { basename as pathBasename, extname, isAbsolute, join, relative, resolve } from 'node:path' +import { basename as pathBasename, extname, isAbsolute, relative, resolve } from 'node:path' import type { MarkdownDocument } from '../../shared/types' +import { + assertMarkdownDocumentPathWithinLimit, + createMarkdownDocumentListingBudget, + retainMarkdownDocument, + type MarkdownDocumentListingLimits +} from '../../shared/markdown-document-listing-limits' +import { + discoverMarkdownRelativePaths, + isMarkdownDocumentPath +} from '../../shared/node-markdown-document-discovery' function normalizeRelativePath(path: string): string { return path.replace(/[\\/]+/g, '/').replace(/^\/+/, '') } export function isMarkdownDocumentName(name: string): boolean { - const extension = extname(name).toLowerCase() - return extension === '.md' || extension === '.mdx' || extension === '.markdown' + return isMarkdownDocumentPath(name) } function basenameFromRelativePath(relativePath: string): string { @@ -39,6 +47,8 @@ export function markdownDocumentFromFilePath( filePath: string, options: { outsideRootRelativePath?: 'basename' | 'relative' } = {} ): MarkdownDocument { + assertMarkdownDocumentPathWithinLimit(rootPath) + assertMarkdownDocumentPathWithinLimit(filePath) const basename = pathBasename(filePath) const extension = extname(basename) const relativePath = @@ -46,18 +56,23 @@ export function markdownDocumentFromFilePath( (options.outsideRootRelativePath === 'basename' ? basename : normalizeRelativePath(relative(rootPath, filePath))) - return { + const document = { filePath, relativePath, basename, name: extension ? basename.slice(0, -extension.length) : basename } + const budget = createMarkdownDocumentListingBudget() + retainMarkdownDocument(budget, document) + return document } export function markdownDocumentFromRelativePath( rootPath: string, relativePath: string ): MarkdownDocument | null { + assertMarkdownDocumentPathWithinLimit(rootPath) + assertMarkdownDocumentPathWithinLimit(relativePath) const normalizedRelativePath = normalizeRelativePath(relativePath) // Why: SSH providers should return root-relative paths; reject escape // segments before building a synthetic absolute path for renderer use. @@ -70,52 +85,42 @@ export function markdownDocumentFromRelativePath( } const extension = extname(basename) const normalizedRoot = rootPath.replace(/[\\/]+$/, '') - return { + const document = { filePath: `${normalizedRoot}/${normalizedRelativePath}`, relativePath: normalizedRelativePath, basename, name: extension ? basename.slice(0, -extension.length) : basename } + const budget = createMarkdownDocumentListingBudget() + retainMarkdownDocument(budget, document) + return document } export function markdownDocumentsFromRelativePaths( rootPath: string, - relativePaths: string[] + relativePaths: readonly string[], + limits: Partial = {} ): MarkdownDocument[] { - return relativePaths - .map((relativePath) => markdownDocumentFromRelativePath(rootPath, relativePath)) - .filter((document): document is MarkdownDocument => document !== null) - .sort((a, b) => a.relativePath.localeCompare(b.relativePath)) -} - -export async function listMarkdownDocuments(rootPath: string): Promise { + const budget = createMarkdownDocumentListingBudget(limits) const documents: MarkdownDocument[] = [] - - async function visitDirectory(dirPath: string): Promise { - const entries = await readdir(dirPath, { withFileTypes: true }) - for (const entry of entries) { - if (entry.isSymbolicLink()) { - continue - } - - const entryPath = join(dirPath, entry.name) - if (entry.isDirectory()) { - if (entry.name === '.git' || entry.name === 'node_modules') { - continue - } - if (entry.name.startsWith('.') && entry.name !== '.github') { - continue - } - await visitDirectory(entryPath) - continue - } - - if (entry.isFile() && isMarkdownDocumentName(entry.name)) { - documents.push(markdownDocumentFromFilePath(rootPath, entryPath)) - } + for (const relativePath of relativePaths) { + const document = markdownDocumentFromRelativePath(rootPath, relativePath) + if (document) { + retainMarkdownDocument(budget, document) + documents.push(document) } } - - await visitDirectory(rootPath) return documents.sort((a, b) => a.relativePath.localeCompare(b.relativePath)) } + +export async function listMarkdownDocuments( + rootPath: string, + limits: Partial = {} +): Promise { + const relativePaths = await discoverMarkdownRelativePaths(rootPath, { + limits, + shouldDescend: (_relativePath, name) => + name !== '.git' && name !== 'node_modules' && (!name.startsWith('.') || name === '.github') + }) + return markdownDocumentsFromRelativePaths(rootPath, relativePaths, limits) +} diff --git a/src/main/providers/filesystem-path-listing-provider.ts b/src/main/providers/filesystem-path-listing-provider.ts index 1cd5c500c8e..951fe981166 100644 --- a/src/main/providers/filesystem-path-listing-provider.ts +++ b/src/main/providers/filesystem-path-listing-provider.ts @@ -3,4 +3,5 @@ export type FilesystemPathListingProvider = { rootPath: string, options?: { excludePaths?: string[]; signal?: AbortSignal; maxResults?: number } ): Promise + listMarkdownDocuments?(rootPath: string): Promise } diff --git a/src/main/providers/ssh-filesystem-provider.ts b/src/main/providers/ssh-filesystem-provider.ts index d9a304af305..231e695069b 100644 --- a/src/main/providers/ssh-filesystem-provider.ts +++ b/src/main/providers/ssh-filesystem-provider.ts @@ -26,6 +26,7 @@ import { routeSshFilesystemWatchNotification } from './ssh-filesystem-watch-noti import type { WorkspaceSpaceDirectoryScanResult } from '../../shared/workspace-space-types' import { isWindowsRemoteHost, type RemoteHostPlatform } from '../ssh/ssh-remote-platform' import { SshFilesystemDirectoryReader } from './ssh-filesystem-directory-reader' +import { requestSshMarkdownDocumentPaths } from './ssh-markdown-document-listing' const WORKSPACE_SPACE_SCAN_TIMEOUT_MS = 130_000 export class SshFilesystemProvider implements IFilesystemProvider { @@ -325,6 +326,8 @@ export class SshFilesystemProvider implements IFilesystemProvider { })) as string[] } + listMarkdownDocuments = (rootPath: string) => requestSshMarkdownDocumentPaths(this.mux, rootPath) + async watch( rootPath: string, callback: (events: FsChangeEvent[]) => void, diff --git a/src/main/providers/ssh-markdown-document-listing.test.ts b/src/main/providers/ssh-markdown-document-listing.test.ts new file mode 100644 index 00000000000..71272b09c5a --- /dev/null +++ b/src/main/providers/ssh-markdown-document-listing.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it, vi } from 'vitest' +import { MarkdownDocumentListingCapacityError } from '../../shared/markdown-document-listing-limits' +import { JsonRpcErrorCode, RelayErrorCode } from '../ssh/relay-protocol' +import { requestSshMarkdownDocumentPaths } from './ssh-markdown-document-listing' + +describe('SSH Markdown document listing', () => { + it('requests producer-filtered Markdown paths', async () => { + const request = vi.fn().mockResolvedValue(['README.md', 'docs/guide.mdx']) + + await expect( + requestSshMarkdownDocumentPaths({ request } as never, '/home/user/project') + ).resolves.toEqual(['README.md', 'docs/guide.mdx']) + expect(request).toHaveBeenCalledWith('fs.listMarkdownDocuments', { + rootPath: '/home/user/project' + }) + }) + + it('surfaces typed producer capacity failures', async () => { + const request = vi.fn().mockRejectedValue( + Object.assign(new Error('capacity'), { + code: RelayErrorCode.MarkdownDocumentListingCapacity + }) + ) + + await expect( + requestSshMarkdownDocumentPaths({ request } as never, '/home/user/project') + ).rejects.toBeInstanceOf(MarkdownDocumentListingCapacityError) + }) + + it('requires reconnect instead of using an unbounded old-relay fallback', async () => { + const request = vi + .fn() + .mockRejectedValue( + Object.assign(new Error('missing'), { code: JsonRpcErrorCode.MethodNotFound }) + ) + + await expect( + requestSshMarkdownDocumentPaths({ request } as never, '/home/user/project') + ).rejects.toThrow('Reconnect the SSH target') + expect(request).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/providers/ssh-markdown-document-listing.ts b/src/main/providers/ssh-markdown-document-listing.ts new file mode 100644 index 00000000000..9061b98aac6 --- /dev/null +++ b/src/main/providers/ssh-markdown-document-listing.ts @@ -0,0 +1,27 @@ +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import { isMethodNotFoundError } from '../ssh/ssh-filesystem-stream-reader' +import { RelayErrorCode } from '../ssh/relay-protocol' +import { MarkdownDocumentListingCapacityError } from '../../shared/markdown-document-listing-limits' + +export async function requestSshMarkdownDocumentPaths( + mux: SshChannelMultiplexer, + rootPath: string +): Promise { + try { + return (await mux.request('fs.listMarkdownDocuments', { rootPath })) as string[] + } catch (error) { + if ( + error instanceof Error && + 'code' in error && + (error as { code?: unknown }).code === RelayErrorCode.MarkdownDocumentListingCapacity + ) { + throw new MarkdownDocumentListingCapacityError() + } + if (isMethodNotFoundError(error)) { + throw new Error( + 'Remote Markdown link discovery is unavailable. Reconnect the SSH target and retry.' + ) + } + throw error + } +} diff --git a/src/main/ssh/relay-protocol.ts b/src/main/ssh/relay-protocol.ts index b8f86cee34e..e317d530ffc 100644 --- a/src/main/ssh/relay-protocol.ts +++ b/src/main/ssh/relay-protocol.ts @@ -43,7 +43,8 @@ export const RelayErrorCode = { PtyAllocationFailed: -33004, DiskFull: -33005, TooManyStreams: -33006, - StreamProtocolError: -33007 + StreamProtocolError: -33007, + MarkdownDocumentListingCapacity: -33008 } as const export const JsonRpcErrorCode = { diff --git a/src/relay/markdown-document-listing.test.ts b/src/relay/markdown-document-listing.test.ts new file mode 100644 index 00000000000..38f93db2ee6 --- /dev/null +++ b/src/relay/markdown-document-listing.test.ts @@ -0,0 +1,93 @@ +import { EventEmitter } from 'node:events' +import type { ChildProcess } from 'node:child_process' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { PassThrough } from 'node:stream' +import { describe, expect, it, vi } from 'vitest' +import { + MARKDOWN_DOCUMENT_LISTING_MAX_DOCUMENTS, + MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES, + MarkdownDocumentListingCapacityError +} from '../shared/markdown-document-listing-limits' +import { + listMarkdownPathsWithRg, + listRelayMarkdownDocumentPaths +} from './markdown-document-listing' + +function successfulChild(output: Buffer): ChildProcess { + const child = Object.assign(new EventEmitter(), { + exitCode: null, + signalCode: null, + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: vi.fn(() => true) + }) as unknown as ChildProcess + queueMicrotask(() => { + child.stdout!.emit('data', output) + child.emit('close', 0, null) + }) + return child +} + +describe('relay Markdown document producer', () => { + it('returns only Markdown paths from a real under-limit workspace', async () => { + const rootPath = await mkdtemp(join(tmpdir(), 'orca-markdown-listing-')) + try { + await mkdir(join(rootPath, 'docs')) + await writeFile(join(rootPath, 'README.md'), 'readme') + await writeFile(join(rootPath, 'docs', 'Guide.MDX'), 'guide') + await writeFile(join(rootPath, 'docs', 'app.ts'), 'code') + + const result = await listRelayMarkdownDocumentPaths(rootPath) + expect(result.sort()).toEqual(['README.md', 'docs/Guide.MDX']) + } finally { + await rm(rootPath, { recursive: true, force: true }) + } + }) + + it('filters a large non-Markdown repository before applying document retention limits', async () => { + const nonMarkdown = Array.from({ length: 100_001 }, (_value, index) => `src/${index}.ts\0`) + const output = Buffer.from(`${nonMarkdown.join('')}README.md\0docs/GUIDE.MDX\0`) + const spawnProcess = vi.fn((_args: string[]) => successfulChild(output)) + + await expect(listMarkdownPathsWithRg('/repo', undefined, spawnProcess)).resolves.toEqual([ + 'README.md', + 'docs/GUIDE.MDX' + ]) + expect(spawnProcess).toHaveBeenCalledTimes(2) + expect(spawnProcess.mock.calls[0]?.[0]).toEqual( + expect.arrayContaining(['--iglob', '*.md', '--null']) + ) + }) + + it('rejects rather than truncating the first document beyond the count limit', async () => { + const output = Buffer.from( + Array.from( + { length: MARKDOWN_DOCUMENT_LISTING_MAX_DOCUMENTS + 1 }, + (_value, index) => `${index}.md\0` + ).join('') + ) + const children: ChildProcess[] = [] + const spawnProcess = vi.fn(() => { + const child = successfulChild(output) + children.push(child) + return child + }) + + await expect(listMarkdownPathsWithRg('/repo', undefined, spawnProcess)).rejects.toBeInstanceOf( + MarkdownDocumentListingCapacityError + ) + expect(children[0]?.kill).toHaveBeenCalled() + expect(spawnProcess).toHaveBeenCalledTimes(1) + }) + + it('bounds an unterminated subprocess path before decoding or retaining it', async () => { + const child = successfulChild(Buffer.alloc(MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES + 1, 0x61)) + + await expect(listMarkdownPathsWithRg('/repo', undefined, () => child)).rejects.toBeInstanceOf( + MarkdownDocumentListingCapacityError + ) + expect(child.kill).toHaveBeenCalled() + }) +}) diff --git a/src/relay/markdown-document-listing.ts b/src/relay/markdown-document-listing.ts new file mode 100644 index 00000000000..a4330f3442b --- /dev/null +++ b/src/relay/markdown-document-listing.ts @@ -0,0 +1,238 @@ +import { spawn, type ChildProcess } from 'node:child_process' +import { checkRgAvailable } from './fs-handler-utils' +import { + buildRgArgsForQuickOpen, + normalizeQuickOpenRgLine, + shouldIncludeQuickOpenPath +} from '../shared/quick-open-filter' +import { + assertMarkdownDocumentPathWithinLimit, + createMarkdownDocumentListingBudget, + isMarkdownDocumentListingCapacityError, + MARKDOWN_DOCUMENT_LISTING_ERROR_MESSAGE, + MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES, + MarkdownDocumentListingCapacityError, + retainMarkdownRelativePath, + visitMarkdownDocumentListingEntry +} from '../shared/markdown-document-listing-limits' +import { + discoverMarkdownRelativePaths, + isMarkdownDocumentPath +} from '../shared/node-markdown-document-discovery' +import { fileListingCancellationError } from '../shared/file-listing-cancellation' +import { RelayErrorCode } from './protocol' +import { GrowingByteBuffer } from '../shared/growing-byte-buffer' + +const MARKDOWN_LISTING_TIMEOUT_MS = 25_000 +const MARKDOWN_GLOBS = ['*.md', '*.mdx', '*.markdown'] + +class MarkdownPathFieldAccumulator { + private readonly field = new GrowingByteBuffer() + + push(chunk: Buffer, onPath: (path: string) => void): boolean { + let cursor = 0 + while (cursor < chunk.length) { + const delimiter = chunk.indexOf(0, cursor) + const end = delimiter === -1 ? chunk.length : delimiter + const segmentBytes = end - cursor + if (this.field.byteLength + segmentBytes > MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES) { + this.clear() + return false + } + if (delimiter !== -1 && this.field.byteLength === 0) { + onPath(chunk.toString('utf8', cursor, end)) + } else if (segmentBytes > 0) { + this.field.append(chunk.subarray(cursor, end)) + if (delimiter !== -1) { + onPath(this.take()) + } + } else if (delimiter !== -1) { + onPath(this.take()) + } + if (delimiter === -1) { + return true + } + cursor = delimiter + 1 + } + return true + } + + finish(): string | null { + return this.field.byteLength > 0 ? this.take() : null + } + + private take(): string { + return this.field.takeString() + } + + private clear(): void { + this.field.clear() + } +} + +function markdownRgArgs(args: string[]): string[] { + const target = args.at(-1) + if (!target) { + throw new Error('Markdown rg scan is missing its search root') + } + const result = [...args.slice(0, -1), '--null'] + for (const glob of MARKDOWN_GLOBS) { + result.push('--iglob', glob) + } + result.push(target) + return result +} + +function relativePathDepth(path: string): number { + let depth = 0 + for (let index = 0; index < path.length; index += 1) { + if (path[index] === '/') { + depth += 1 + } + } + return depth +} + +export async function listMarkdownPathsWithRg( + rootPath: string, + signal?: AbortSignal, + spawnProcess: (args: string[]) => ChildProcess = (args) => + spawn('rg', args, { cwd: rootPath, stdio: ['ignore', 'pipe', 'pipe'] }) +): Promise { + const paths = new Set() + const budget = createMarkdownDocumentListingBudget() + assertMarkdownDocumentPathWithinLimit(rootPath, budget.limits.maxPathBytes) + const passes = buildRgArgsForQuickOpen({ + searchRoot: '.', + excludePathPrefixes: [], + forceSlashSeparator: true + }) + + const runPass = (args: string[]): Promise => + new Promise((resolve, reject) => { + if (signal?.aborted) { + reject(fileListingCancellationError(signal)) + return + } + const child = spawnProcess(markdownRgArgs(args)) + const fields = new MarkdownPathFieldAccumulator() + let done = false + let parseablePathCount = 0 + let timer: ReturnType | null = null + + const cleanup = (): void => { + if (timer) { + clearTimeout(timer) + timer = null + } + child.stdout?.off('data', onStdout) + child.stderr?.off('data', onStderr) + child.off('error', onError) + child.off('close', onClose) + signal?.removeEventListener('abort', onAbort) + } + const finish = (error?: Error, kill = false): void => { + if (done) { + return + } + done = true + cleanup() + if (kill && child.exitCode === null && child.signalCode === null) { + child.kill() + } + if (error) { + reject(error) + } else { + resolve() + } + } + const processPath = (rawPath: string): void => { + const relativePath = normalizeQuickOpenRgLine(rawPath, { kind: 'cwd-relative' }) + if (relativePath === null) { + return + } + parseablePathCount += 1 + if ( + paths.has(relativePath) || + !shouldIncludeQuickOpenPath(relativePath) || + !isMarkdownDocumentPath(relativePath) + ) { + return + } + visitMarkdownDocumentListingEntry(budget, relativePath, relativePathDepth(relativePath)) + retainMarkdownRelativePath(budget, rootPath, relativePath) + paths.add(relativePath) + } + const onStdout = (chunk: Buffer): void => { + try { + if (!fields.push(chunk, processPath)) { + finish(new MarkdownDocumentListingCapacityError(), true) + } + } catch (error) { + finish(error instanceof Error ? error : new Error(String(error)), true) + } + } + const onStderr = (): void => {} + const onError = (error: Error): void => finish(error) + const onClose = (code: number | null, exitSignal: NodeJS.Signals | null): void => { + if (exitSignal) { + finish(new Error(`rg killed by ${exitSignal}`)) + return + } + try { + const trailingPath = fields.finish() + if (trailingPath !== null) { + processPath(trailingPath) + } + } catch (error) { + finish(error instanceof Error ? error : new Error(String(error))) + return + } + if (code === 0 || code === 1 || (code === 2 && parseablePathCount > 0)) { + finish() + } else { + finish(new Error(`rg exited with code ${code}`)) + } + } + const onAbort = (): void => finish(fileListingCancellationError(signal), true) + + child.stdout?.on('data', onStdout) + child.stderr?.on('data', onStderr) + child.once('error', onError) + child.once('close', onClose) + signal?.addEventListener('abort', onAbort, { once: true }) + timer = setTimeout( + () => finish(new Error('Markdown document listing timed out'), true), + MARKDOWN_LISTING_TIMEOUT_MS + ) + }) + + await runPass(passes.primary) + await runPass(passes.ignoredPass) + return Array.from(paths) +} + +export async function listRelayMarkdownDocumentPaths( + rootPath: string, + signal?: AbortSignal +): Promise { + try { + if (await checkRgAvailable()) { + return await listMarkdownPathsWithRg(rootPath, signal) + } + return await discoverMarkdownRelativePaths(rootPath, { + ignoreNestedDirectoryErrors: true, + shouldDescend: (relativePath) => shouldIncludeQuickOpenPath(relativePath), + signal + }) + } catch (error) { + if (!isMarkdownDocumentListingCapacityError(error)) { + throw error + } + const relayError = new Error(MARKDOWN_DOCUMENT_LISTING_ERROR_MESSAGE) as Error & { + code: number + } + relayError.code = RelayErrorCode.MarkdownDocumentListingCapacity + throw relayError + } +} diff --git a/src/relay/protocol.ts b/src/relay/protocol.ts index 4df0f7bd9b5..ddaf8863b7a 100644 --- a/src/relay/protocol.ts +++ b/src/relay/protocol.ts @@ -82,7 +82,8 @@ export type GitResponseStreamMarker = { export const RelayErrorCode = { TooManyStreams: -33006, - StreamProtocolError: -33007 + StreamProtocolError: -33007, + MarkdownDocumentListingCapacity: -33008 } as const export type JsonRpcRequest = { diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index 417aaccf62e..58049f95ea8 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -84,6 +84,7 @@ type MonacoEditorProps = { revealMatchLength?: number markdownDocuments?: MarkdownDocument[] worktreeId?: string + runtimeEnvironmentId?: string | null markdownAnnotationsEnabled?: boolean conflictDecorationsEnabled?: boolean readOnly?: boolean @@ -110,6 +111,7 @@ export default function MonacoEditor({ revealMatchLength, markdownDocuments, worktreeId, + runtimeEnvironmentId, markdownAnnotationsEnabled = false, conflictDecorationsEnabled = false, readOnly = false, @@ -208,11 +210,15 @@ export default function MonacoEditor({ return } if (language === 'markdown' && markdownDocuments) { - setMarkdownDocCompletionDocuments(modelKey, markdownDocuments) + setMarkdownDocCompletionDocuments( + modelKey, + JSON.stringify([runtimeEnvironmentId ?? '', worktreeId ?? modelKey]), + markdownDocuments + ) } else { clearMarkdownDocCompletionDocuments(modelKey) } - }, [language, markdownDocuments]) + }, [language, markdownDocuments, runtimeEnvironmentId, worktreeId]) const shouldShowMarkdownAnnotations = markdownAnnotationsEnabled && language === 'markdown' && Boolean(worktreeId) diff --git a/src/renderer/src/components/editor/markdown-doc-links.test.ts b/src/renderer/src/components/editor/markdown-doc-links.test.ts index d5cfaade6e7..0c4b5c00912 100644 --- a/src/renderer/src/components/editor/markdown-doc-links.test.ts +++ b/src/renderer/src/components/editor/markdown-doc-links.test.ts @@ -71,6 +71,10 @@ describe('splitMarkdownDocLinkText', () => { }) describe('resolveMarkdownDocLink', () => { + it('shares one immutable index for consumers of the same document snapshot', () => { + expect(createMarkdownDocumentIndex(documents)).toBe(createMarkdownDocumentIndex(documents)) + }) + it('resolves basename links', () => { const result = resolveMarkdownDocLink('setup-guide', createMarkdownDocumentIndex(documents)) expect(result.status).toBe('resolved') diff --git a/src/renderer/src/components/editor/markdown-doc-links.ts b/src/renderer/src/components/editor/markdown-doc-links.ts index 82cd7133b38..37c8ce93836 100644 --- a/src/renderer/src/components/editor/markdown-doc-links.ts +++ b/src/renderer/src/components/editor/markdown-doc-links.ts @@ -101,7 +101,13 @@ function resolveMatches(matches: MarkdownDocument[] | undefined): MarkdownDocLin : { status: 'ambiguous', matches } } +const markdownDocumentIndexes = new WeakMap() + export function createMarkdownDocumentIndex(documents: MarkdownDocument[]): MarkdownDocumentIndex { + const cached = markdownDocumentIndexes.get(documents) + if (cached) { + return cached + } const byName = new Map() const byRelativePath = new Map() const byRelativePathWithoutExtension = new Map() @@ -116,7 +122,9 @@ export function createMarkdownDocumentIndex(documents: MarkdownDocument[]): Mark ) } - return { byName, byRelativePath, byRelativePathWithoutExtension } + const index = { byName, byRelativePath, byRelativePathWithoutExtension } + markdownDocumentIndexes.set(documents, index) + return index } export function resolveMarkdownDocLink( diff --git a/src/renderer/src/components/editor/markdown-document-list-request.test.ts b/src/renderer/src/components/editor/markdown-document-list-request.test.ts index 085b8599fb8..96e6bd1c528 100644 --- a/src/renderer/src/components/editor/markdown-document-list-request.test.ts +++ b/src/renderer/src/components/editor/markdown-document-list-request.test.ts @@ -3,8 +3,10 @@ import type { MarkdownDocument } from '../../../../shared/types' import type { RuntimeFileOperationArgs } from '@/runtime/runtime-file-client' import { getMarkdownDocumentListRequestKey, + MARKDOWN_DOCUMENT_LIST_MAX_IN_FLIGHT, requestSharedMarkdownDocumentList } from './markdown-document-list-request' +import { MarkdownDocumentListingCapacityError } from '../../../../shared/markdown-document-listing-limits' function context(overrides: Partial = {}): RuntimeFileOperationArgs { return { @@ -173,4 +175,29 @@ describe('shared Markdown document list requests', () => { ) expect(load).toHaveBeenCalledTimes(2) }) + + it('rejects the first distinct scan beyond the global in-flight cap', async () => { + const pending = Array.from({ length: MARKDOWN_DOCUMENT_LIST_MAX_IN_FLIGHT }, () => + deferred() + ) + const load = vi.fn((_context: RuntimeFileOperationArgs, rootPath: string) => { + const index = Number(rootPath.slice('/route-'.length)) + return pending[index].promise + }) + const requests = pending.map((_entry, index) => + requestSharedMarkdownDocumentList(context(), `/route-${index}`, {}, load) + ) + + await expect( + requestSharedMarkdownDocumentList(context(), '/one-too-many', {}, load) + ).rejects.toBeInstanceOf(MarkdownDocumentListingCapacityError) + expect(load).toHaveBeenCalledTimes(MARKDOWN_DOCUMENT_LIST_MAX_IN_FLIGHT) + + for (const entry of pending) { + entry.resolve([]) + } + await expect(Promise.all(requests)).resolves.toEqual( + Array.from({ length: MARKDOWN_DOCUMENT_LIST_MAX_IN_FLIGHT }, () => []) + ) + }) }) diff --git a/src/renderer/src/components/editor/markdown-document-list-request.ts b/src/renderer/src/components/editor/markdown-document-list-request.ts index 7e9cc0f32b3..9d55a502637 100644 --- a/src/renderer/src/components/editor/markdown-document-list-request.ts +++ b/src/renderer/src/components/editor/markdown-document-list-request.ts @@ -3,6 +3,7 @@ import { listRuntimeMarkdownDocuments, type RuntimeFileOperationArgs } from '@/runtime/runtime-file-client' +import { MarkdownDocumentListingCapacityError } from '../../../../shared/markdown-document-listing-limits' type MarkdownDocumentListLoader = ( context: RuntimeFileOperationArgs, @@ -19,7 +20,9 @@ type InFlightMarkdownDocumentList = { } const MARKDOWN_DOCUMENT_LIST_JOIN_WINDOW_MS = 30_000 +export const MARKDOWN_DOCUMENT_LIST_MAX_IN_FLIGHT = 16 const inFlightMarkdownDocumentLists = new Map() +let activeMarkdownDocumentListLoads = 0 export function getMarkdownDocumentListRequestKey( context: RuntimeFileOperationArgs, @@ -54,10 +57,22 @@ export function requestSharedMarkdownDocumentList( if (existing && !options.requireFresh) { return existing.request } + if (activeMarkdownDocumentListLoads >= MARKDOWN_DOCUMENT_LIST_MAX_IN_FLIGHT) { + return Promise.reject(new MarkdownDocumentListingCapacityError()) + } // Why: split Markdown panes mount together and otherwise launch identical // whole-worktree local/SSH scans; mutation refreshes bypass older snapshots. - const request = load(context, rootPath).finally(() => { + activeMarkdownDocumentListLoads += 1 + let loaded: Promise + try { + loaded = load(context, rootPath) + } catch (error) { + activeMarkdownDocumentListLoads -= 1 + return Promise.reject(error) + } + const request = loaded.finally(() => { + activeMarkdownDocumentListLoads -= 1 if (inFlightMarkdownDocumentLists.get(key)?.request === request) { inFlightMarkdownDocumentLists.delete(key) } diff --git a/src/renderer/src/components/editor/markdown-document-worktree-retention.test.ts b/src/renderer/src/components/editor/markdown-document-worktree-retention.test.ts new file mode 100644 index 00000000000..b20ae5104ec --- /dev/null +++ b/src/renderer/src/components/editor/markdown-document-worktree-retention.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from 'vitest' +import type { MarkdownDocument } from '../../../../shared/types' +import { + retainMarkdownDocumentWorktreeSnapshot, + type MarkdownDocumentWorktreeSnapshot +} from './markdown-document-worktree-retention' + +function documents(name: string): MarkdownDocument[] { + return [ + { + filePath: `/repo/${name}.md`, + relativePath: `${name}.md`, + basename: `${name}.md`, + name + } + ] +} + +describe('Markdown document worktree retention', () => { + it('preserves recent under-limit worktrees and refreshes their LRU position', () => { + let snapshots = new Map() + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, 'a', documents('a'), { + maxSnapshots: 2, + maxRetainedBytes: 10_000 + }) + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, 'b', documents('b'), { + maxSnapshots: 2, + maxRetainedBytes: 10_000 + }) + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, 'a', documents('a-new'), { + maxSnapshots: 2, + maxRetainedBytes: 10_000 + }) + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, 'c', documents('c'), { + maxSnapshots: 2, + maxRetainedBytes: 10_000 + }) + + expect(Array.from(snapshots.keys())).toEqual(['a', 'c']) + expect(snapshots.get('a')?.documents[0]?.name).toBe('a-new') + }) + + it('retains at most 8 worktree snapshots by default', () => { + // Why: the cases here pass explicit limits, which never exercise the shipped defaults. Asserted + // against the literal ceiling so raising the constant fails rather than silently retaining more. + let snapshots = new Map() + for (let index = 0; index < 12; index += 1) { + snapshots = retainMarkdownDocumentWorktreeSnapshot( + snapshots, + `worktree-${index}`, + documents(`doc-${index}`) + ) + } + + expect(snapshots.size).toBe(8) + expect(Array.from(snapshots.keys())).toEqual( + Array.from({ length: 8 }, (_unused, index) => `worktree-${index + 4}`) + ) + }) + + it('caps aggregate retained bytes at 32 MiB by default', () => { + // A single snapshot cannot exceed the 8 MiB per-listing ceiling, so the aggregate cap only + // binds across several worktrees — which is exactly the case it exists for. + const largeDocuments = Array.from({ length: 8_000 }, (_unused, index) => ({ + filePath: `/repo/${'nested/'.repeat(20)}doc-${index}.md`, + relativePath: `${'nested/'.repeat(20)}doc-${index}.md`, + basename: `doc-${index}.md`, + name: `doc-${index}` + })) + + let snapshots = new Map() + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, 'w-0', largeDocuments) + const perSnapshotBytes = snapshots.get('w-0')?.retainedBytes ?? 0 + // Five of these exceed 32 MiB while each stays under the per-listing ceiling. + expect(perSnapshotBytes).toBeGreaterThan(6 * 1024 * 1024) + + for (let index = 1; index < 5; index += 1) { + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, `w-${index}`, largeDocuments) + } + + // Well under the 8-snapshot cap, so eviction here can only come from the byte ceiling. + expect(snapshots.size).toBeLessThan(5) + const retainedBytes = Array.from(snapshots.values()).reduce( + (total, snapshot) => total + snapshot.retainedBytes, + 0 + ) + expect(retainedBytes).toBeLessThanOrEqual(32 * 1024 * 1024) + }) + + it('evicts oldest snapshots when their aggregate byte budget is exceeded', () => { + let snapshots = new Map() + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, 'a', documents('a'), { + maxSnapshots: 10, + maxRetainedBytes: 10_000 + }) + const oneSnapshotBytes = snapshots.get('a')?.retainedBytes ?? 0 + snapshots = retainMarkdownDocumentWorktreeSnapshot(snapshots, 'b', documents('b'), { + maxSnapshots: 10, + maxRetainedBytes: oneSnapshotBytes + }) + + expect(Array.from(snapshots.keys())).toEqual(['b']) + }) +}) diff --git a/src/renderer/src/components/editor/markdown-document-worktree-retention.ts b/src/renderer/src/components/editor/markdown-document-worktree-retention.ts new file mode 100644 index 00000000000..4b3db84647f --- /dev/null +++ b/src/renderer/src/components/editor/markdown-document-worktree-retention.ts @@ -0,0 +1,41 @@ +import type { MarkdownDocument } from '../../../../shared/types' +import { assertMarkdownDocumentsWithinLimit } from '../../../../shared/markdown-document-listing-limits' + +export const MARKDOWN_DOCUMENT_WORKTREE_MAX_SNAPSHOTS = 8 +export const MARKDOWN_DOCUMENT_WORKTREE_MAX_RETAINED_BYTES = 32 * 1024 * 1024 + +export type MarkdownDocumentWorktreeSnapshot = { + documents: MarkdownDocument[] + retainedBytes: number +} + +export function retainMarkdownDocumentWorktreeSnapshot( + previous: ReadonlyMap, + worktreeId: string, + documents: MarkdownDocument[], + limits: { maxSnapshots: number; maxRetainedBytes: number } = { + maxSnapshots: MARKDOWN_DOCUMENT_WORKTREE_MAX_SNAPSHOTS, + maxRetainedBytes: MARKDOWN_DOCUMENT_WORKTREE_MAX_RETAINED_BYTES + } +): Map { + const next = new Map(previous) + next.delete(worktreeId) + next.set(worktreeId, { + documents, + retainedBytes: assertMarkdownDocumentsWithinLimit(documents) + }) + + let retainedBytes = 0 + for (const snapshot of next.values()) { + retainedBytes += snapshot.retainedBytes + } + while (next.size > limits.maxSnapshots || retainedBytes > limits.maxRetainedBytes) { + const oldestKey = next.keys().next().value + if (typeof oldestKey !== 'string') { + break + } + retainedBytes -= next.get(oldestKey)?.retainedBytes ?? 0 + next.delete(oldestKey) + } + return next +} diff --git a/src/renderer/src/components/editor/markdown-export-extract.test.ts b/src/renderer/src/components/editor/markdown-export-extract.test.ts index f10a8289c44..9494b10accf 100644 --- a/src/renderer/src/components/editor/markdown-export-extract.test.ts +++ b/src/renderer/src/components/editor/markdown-export-extract.test.ts @@ -1,7 +1,7 @@ // @vitest-environment happy-dom import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { getActiveMarkdownExportPayload } from './markdown-export-extract' +import { getActiveMarkdownExportPayload, inlineBlobImageSources } from './markdown-export-extract' vi.mock('@/store', () => ({ useAppStore: { @@ -14,10 +14,11 @@ describe('getActiveMarkdownExportPayload', () => { vi.clearAllMocks() vi.stubGlobal( 'fetch', - vi.fn().mockResolvedValue({ - ok: true, - blob: async () => new Blob([new Uint8Array([1, 2, 3])], { type: 'image/png' }) - }) + vi + .fn() + .mockResolvedValue( + new Response(new Uint8Array([1, 2, 3]), { headers: { 'content-type': 'image/png' } }) + ) ) const { useAppStore } = await import('@/store') vi.mocked(useAppStore.getState).mockReturnValue({ @@ -66,4 +67,30 @@ describe('getActiveMarkdownExportPayload', () => { }) ).rejects.toThrow('Failed to inline image for PDF export') }) + + it('rejects streamed image bytes before the rendered fragment can exceed its cap', async () => { + let cancelled = false + vi.mocked(fetch).mockResolvedValue( + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(12)) + controller.enqueue(new Uint8Array(12)) + }, + cancel() { + cancelled = true + } + }), + { headers: { 'content-type': 'image/png' } } + ) + ) + const root = document.createElement('div') + root.innerHTML = '' + const prefixOnly = ''.length + + await expect(inlineBlobImageSources(root, prefixOnly + 16)).rejects.toThrow( + 'HTML export exceeds the PDF memory limit' + ) + expect(cancelled).toBe(true) + }) }) diff --git a/src/renderer/src/components/editor/markdown-export-extract.ts b/src/renderer/src/components/editor/markdown-export-extract.ts index a0ec5ebb2fe..23d73fa27e6 100644 --- a/src/renderer/src/components/editor/markdown-export-extract.ts +++ b/src/renderer/src/components/editor/markdown-export-extract.ts @@ -1,5 +1,15 @@ import { useAppStore } from '@/store' import { detectLanguage } from '@/lib/language-detect' +import { + assertHtmlToPdfInputWithinMemoryLimit, + HTML_TO_PDF_MAX_INPUT_BYTES, + HTML_TO_PDF_MEMORY_LIMIT_ERROR +} from '../../../../shared/html-to-pdf-memory-limit' +import { + FetchResponseBodyTooLargeError, + readFetchResponseBytesWithinLimit +} from '../../../../shared/fetch-response-body' +import { measureUtf8ByteLength } from '../../../../shared/memory-safety/utf8-byte-limits' import { buildMarkdownExportHtml } from './markdown-export-html' export type MarkdownExportPayload = { @@ -65,6 +75,15 @@ export async function getActiveMarkdownExportPayload({ return null } + const title = basenameWithoutExt(activeFile.relativePath || activeFile.filePath) + const wrapper = buildMarkdownExportHtml({ title, renderedHtml: '' }) + const wrapperBytes = measureUtf8ByteLength(wrapper, { + stopAfterBytes: HTML_TO_PDF_MAX_INPUT_BYTES + }) + if (wrapperBytes.exceededLimit) { + throw new Error(HTML_TO_PDF_MEMORY_LIMIT_ERROR) + } + const clone = subtree.cloneNode(true) as Element for (const selector of UI_ONLY_SELECTORS) { for (const node of clone.querySelectorAll(selector)) { @@ -73,51 +92,77 @@ export async function getActiveMarkdownExportPayload({ } // Why: local-image previews use renderer-scoped blob URLs; the hidden PDF // window cannot dereference them, so embed the bytes before export. - await inlineBlobImageSources(clone) + await inlineBlobImageSources(clone, HTML_TO_PDF_MAX_INPUT_BYTES - wrapperBytes.byteLength) const renderedHtml = clone.innerHTML.trim() if (!renderedHtml) { return null } - const title = basenameWithoutExt(activeFile.relativePath || activeFile.filePath) const html = buildMarkdownExportHtml({ title, renderedHtml }) + assertHtmlToPdfInputWithinMemoryLimit(html) return { title, html } } -async function inlineBlobImageSources(root: Element): Promise { - const images = Array.from(root.querySelectorAll('img[src^="blob:"]')) - await Promise.all( - images.map(async (image) => { - const src = image.getAttribute('src') - if (!src) { - return - } - image.setAttribute('src', await readBlobImageAsDataUrl(src)) - }) - ) +export async function inlineBlobImageSources( + root: Element, + maxFragmentBytes: number +): Promise { + for (const image of root.querySelectorAll('img[src^="blob:"]')) { + const src = image.getAttribute('src') + if (!src) { + continue + } + await inlineBlobImageSource(root, image, src, maxFragmentBytes) + } } -async function readBlobImageAsDataUrl(src: string): Promise { +async function inlineBlobImageSource( + root: Element, + image: HTMLImageElement, + src: string, + maxFragmentBytes: number +): Promise { try { const response = await fetch(src) if (!response.ok) { throw new Error('Unable to fetch blob image') } - const blob = await response.blob() - const bytes = new Uint8Array(await blob.arrayBuffer()) - return `data:${blob.type || 'application/octet-stream'};base64,${bytesToBase64(bytes)}` + const mediaType = normalizedMediaType(response.headers.get('content-type')) + const prefix = `data:${mediaType};base64,` + image.setAttribute('src', prefix) + const prefixMeasurement = measureUtf8ByteLength(root.innerHTML, { + stopAfterBytes: maxFragmentBytes + }) + if (prefixMeasurement.exceededLimit) { + throw new Error(HTML_TO_PDF_MEMORY_LIMIT_ERROR) + } + const availableBase64Characters = maxFragmentBytes - prefixMeasurement.byteLength + const maxImageBytes = Math.floor(availableBase64Characters / 4) * 3 + const bytes = await readFetchResponseBytesWithinLimit(response, maxImageBytes) + image.setAttribute('src', `${prefix}${bytesToBase64(bytes)}`) } catch (error) { + if ( + error instanceof FetchResponseBodyTooLargeError || + (error instanceof Error && error.message === HTML_TO_PDF_MEMORY_LIMIT_ERROR) + ) { + throw new Error(HTML_TO_PDF_MEMORY_LIMIT_ERROR) + } const message = error instanceof Error ? error.message : String(error) throw new Error(`Failed to inline image for PDF export: ${message}`) } } +function normalizedMediaType(value: string | null): string { + return new Blob([], { type: value ?? '' }).type || 'application/octet-stream' +} + function bytesToBase64(bytes: Uint8Array): string { - let binary = '' - const chunkSize = 0x8000 + const chunks: string[] = [] + const chunkSize = 3 * 8192 for (let index = 0; index < bytes.length; index += chunkSize) { - binary += String.fromCharCode(...bytes.subarray(index, index + chunkSize)) + const binary = String.fromCharCode(...bytes.subarray(index, index + chunkSize)) + chunks.push(btoa(binary)) } - return btoa(binary) + return chunks.join('') } diff --git a/src/renderer/src/components/editor/monaco-markdown-doc-completions.test.ts b/src/renderer/src/components/editor/monaco-markdown-doc-completions.test.ts new file mode 100644 index 00000000000..a13470cb515 --- /dev/null +++ b/src/renderer/src/components/editor/monaco-markdown-doc-completions.test.ts @@ -0,0 +1,68 @@ +import { afterEach, describe, expect, it } from 'vitest' +import type { MarkdownDocument } from '../../../../shared/types' +import { assertMarkdownDocumentsWithinLimit } from '../../../../shared/markdown-document-listing-limits' +import { + clearMarkdownDocCompletionDocuments, + getMarkdownCompletionRetentionForTests, + MARKDOWN_COMPLETION_MAX_MODELS, + MARKDOWN_COMPLETION_MAX_SCOPES, + resetMarkdownCompletionRetentionForTests, + setMarkdownDocCompletionDocuments +} from './monaco-markdown-doc-completions' + +function documents(scope: string): MarkdownDocument[] { + return [ + { + filePath: `/repo/${scope}.md`, + relativePath: `${scope}.md`, + basename: `${scope}.md`, + name: scope + } + ] +} + +afterEach(() => { + resetMarkdownCompletionRetentionForTests() +}) + +describe('Monaco Markdown completion retention', () => { + it('stores one document snapshot for every mounted model in the same worktree scope', () => { + setMarkdownDocCompletionDocuments('model-a', 'worktree-a', documents('old')) + const freshDocuments = documents('fresh') + setMarkdownDocCompletionDocuments('model-b', 'worktree-a', freshDocuments) + + expect(getMarkdownCompletionRetentionForTests()).toEqual({ + models: 2, + scopes: 1, + retainedBytes: assertMarkdownDocumentsWithinLimit(freshDocuments) + }) + + clearMarkdownDocCompletionDocuments('model-a') + expect(getMarkdownCompletionRetentionForTests()).toMatchObject({ models: 1, scopes: 1 }) + clearMarkdownDocCompletionDocuments('model-b') + expect(getMarkdownCompletionRetentionForTests()).toEqual({ + models: 0, + scopes: 0, + retainedBytes: 0 + }) + }) + + it('evicts oldest scopes and model associations at their exact caps', () => { + for (let index = 0; index <= MARKDOWN_COMPLETION_MAX_SCOPES; index += 1) { + setMarkdownDocCompletionDocuments(`model-${index}`, `scope-${index}`, documents(`${index}`)) + } + expect(getMarkdownCompletionRetentionForTests()).toMatchObject({ + models: MARKDOWN_COMPLETION_MAX_SCOPES, + scopes: MARKDOWN_COMPLETION_MAX_SCOPES + }) + + resetMarkdownCompletionRetentionForTests() + for (let index = 0; index <= MARKDOWN_COMPLETION_MAX_MODELS; index += 1) { + setMarkdownDocCompletionDocuments(`model-${index}`, 'shared-scope', documents('shared')) + } + expect(getMarkdownCompletionRetentionForTests()).toMatchObject({ + models: MARKDOWN_COMPLETION_MAX_MODELS, + scopes: 1 + }) + }) +}) diff --git a/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts b/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts index 882b011e9b3..d457004553f 100644 --- a/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts +++ b/src/renderer/src/components/editor/monaco-markdown-doc-completions.ts @@ -5,12 +5,72 @@ import { getMarkdownDocCompletionContext, getMarkdownDocCompletionDocuments } from './markdown-doc-completions' +import { assertMarkdownDocumentsWithinLimit } from '../../../../shared/markdown-document-listing-limits' type MonacoApi = Parameters[1] +type CompletionScope = { + documents: MarkdownDocument[] + modelKeys: Set + retainedBytes: number +} + +export const MARKDOWN_COMPLETION_MAX_MODELS = 256 +export const MARKDOWN_COMPLETION_MAX_SCOPES = 32 +export const MARKDOWN_COMPLETION_MAX_RETAINED_BYTES = 64 * 1024 * 1024 + let provider: IDisposable | null = null let providerMonaco: MonacoApi | null = null -const documentsByModel = new Map() +const scopeKeyByModel = new Map() +const completionScopes = new Map() +let retainedBytes = 0 + +function deleteCompletionScope(scopeKey: string, scope: CompletionScope): void { + completionScopes.delete(scopeKey) + retainedBytes -= scope.retainedBytes + for (const modelKey of scope.modelKeys) { + scopeKeyByModel.delete(modelKey) + } +} + +function removeModel(modelKey: string): void { + const scopeKey = scopeKeyByModel.get(modelKey) + if (!scopeKey) { + return + } + scopeKeyByModel.delete(modelKey) + const scope = completionScopes.get(scopeKey) + scope?.modelKeys.delete(modelKey) + if (scope && scope.modelKeys.size === 0) { + deleteCompletionScope(scopeKey, scope) + } +} + +function enforceCompletionRetentionLimits(): void { + while (scopeKeyByModel.size > MARKDOWN_COMPLETION_MAX_MODELS) { + const oldestModelKey = scopeKeyByModel.keys().next().value + if (typeof oldestModelKey !== 'string') { + break + } + removeModel(oldestModelKey) + } + while ( + completionScopes.size > MARKDOWN_COMPLETION_MAX_SCOPES || + retainedBytes > MARKDOWN_COMPLETION_MAX_RETAINED_BYTES + ) { + const oldest = completionScopes.entries().next().value + if (!oldest) { + break + } + deleteCompletionScope(oldest[0], oldest[1]) + } +} + +function clearCompletionRetention(): void { + scopeKeyByModel.clear() + completionScopes.clear() + retainedBytes = 0 +} export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { // Why: if Monaco was torn down and re-created (e.g. window reload), the old @@ -21,7 +81,7 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { } if (provider) { provider.dispose() - documentsByModel.clear() + clearCompletionRetention() } providerMonaco = monaco @@ -34,7 +94,8 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { return { suggestions: [] } } - const documents = documentsByModel.get(model.uri.toString()) ?? [] + const scopeKey = scopeKeyByModel.get(model.uri.toString()) + const documents = scopeKey ? (completionScopes.get(scopeKey)?.documents ?? []) : [] const suffix = line.slice(position.column - 1) const range = { startLineNumber: position.lineNumber, @@ -60,11 +121,49 @@ export function ensureMarkdownDocCompletionProvider(monaco: MonacoApi): void { export function setMarkdownDocCompletionDocuments( modelKey: string, + scopeKey: string, documents: MarkdownDocument[] ): void { - documentsByModel.set(modelKey, documents) + removeModel(modelKey) + let nextRetainedBytes: number + try { + nextRetainedBytes = assertMarkdownDocumentsWithinLimit(documents) + } catch { + return + } + + let scope = completionScopes.get(scopeKey) + if (scope) { + completionScopes.delete(scopeKey) + retainedBytes -= scope.retainedBytes + scope.documents = documents + scope.retainedBytes = nextRetainedBytes + } else { + scope = { documents, modelKeys: new Set(), retainedBytes: nextRetainedBytes } + } + scope.modelKeys.add(modelKey) + completionScopes.set(scopeKey, scope) + scopeKeyByModel.set(modelKey, scopeKey) + retainedBytes += nextRetainedBytes + enforceCompletionRetentionLimits() } export function clearMarkdownDocCompletionDocuments(modelKey: string): void { - documentsByModel.delete(modelKey) + removeModel(modelKey) +} + +export function getMarkdownCompletionRetentionForTests(): { + models: number + scopes: number + retainedBytes: number +} { + return { + models: scopeKeyByModel.size, + scopes: completionScopes.size, + retainedBytes + } +} + +export function resetMarkdownCompletionRetentionForTests(): void { + clearCompletionRetention() } diff --git a/src/renderer/src/components/editor/rich-markdown-local-image.test.ts b/src/renderer/src/components/editor/rich-markdown-local-image.test.ts index e343e918619..0d4a629a6bb 100644 --- a/src/renderer/src/components/editor/rich-markdown-local-image.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-local-image.test.ts @@ -22,7 +22,7 @@ describe('rich markdown local images', () => { ...globalThis.window.api, fs: { readFile: vi.fn().mockResolvedValue({ - content: 'AA==', + content: 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAAB', isBinary: true, mimeType: 'image/png' }) diff --git a/src/renderer/src/components/editor/useMarkdownDocuments.test.tsx b/src/renderer/src/components/editor/useMarkdownDocuments.test.tsx new file mode 100644 index 00000000000..b0b41dd9799 --- /dev/null +++ b/src/renderer/src/components/editor/useMarkdownDocuments.test.tsx @@ -0,0 +1,96 @@ +// @vitest-environment happy-dom +import { act, renderHook, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { MarkdownDocument } from '../../../../shared/types' +import { MarkdownDocumentListingCapacityError } from '../../../../shared/markdown-document-listing-limits' +import type { OpenFile } from '@/store/slices/editor' +import { useMarkdownDocuments } from './useMarkdownDocuments' + +const mocks = vi.hoisted(() => ({ + list: vi.fn(), + openFile: vi.fn(), + openMarkdownPreview: vi.fn(), + toastError: vi.fn() +})) + +vi.mock('sonner', () => ({ toast: { error: mocks.toastError } })) +vi.mock('@/lib/connection-context', () => ({ getConnectionId: () => null })) +vi.mock('@/runtime/runtime-file-client', () => ({ statRuntimePath: vi.fn() })) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + settingsForRuntimeOwner: () => ({ activeRuntimeEnvironmentId: null }) +})) +vi.mock('./markdown-document-list-request', () => ({ + requestSharedMarkdownDocumentList: mocks.list +})) +vi.mock('./markdown-document-worktree-path-selector', () => ({ + selectMarkdownDocumentWorktreePath: (_state: unknown, worktreeId: string) => `/repo/${worktreeId}` +})) +vi.mock('@/store', () => { + const state = { + settings: {}, + openFile: mocks.openFile, + openMarkdownPreview: mocks.openMarkdownPreview + } + const useAppStore = Object.assign( + (selector: (value: typeof state) => unknown) => selector(state), + { getState: () => state } + ) + return { useAppStore } +}) + +function file(worktreeId: string): OpenFile { + return { + id: `/repo/${worktreeId}/README.md`, + filePath: `/repo/${worktreeId}/README.md`, + relativePath: 'README.md', + worktreeId, + language: 'markdown', + isDirty: false, + mode: 'edit' + } +} + +function deferred(): { + promise: Promise + reject: (error: Error) => void + resolve: (value: T) => void +} { + let reject!: (error: Error) => void + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise + reject = rejectPromise + }) + return { promise, reject, resolve } +} + +afterEach(() => { + vi.clearAllMocks() + vi.restoreAllMocks() +}) + +describe('useMarkdownDocuments', () => { + it('does not toast when a superseded worktree scan later exceeds capacity', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + const first = deferred() + const second = deferred() + mocks.list.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise) + + const hook = renderHook( + ({ activeFile }) => useMarkdownDocuments(activeFile, true, 'source', vi.fn()), + { initialProps: { activeFile: file('first') } } + ) + await waitFor(() => expect(mocks.list).toHaveBeenCalledTimes(1)) + + hook.rerender({ activeFile: file('second') }) + await waitFor(() => expect(mocks.list).toHaveBeenCalledTimes(2)) + + await act(async () => { + first.reject(new MarkdownDocumentListingCapacityError()) + second.resolve([]) + await Promise.allSettled([first.promise, second.promise]) + }) + + expect(mocks.toastError).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/editor/useMarkdownDocuments.ts b/src/renderer/src/components/editor/useMarkdownDocuments.ts index 5e8e5a17fb8..842f826a62a 100644 --- a/src/renderer/src/components/editor/useMarkdownDocuments.ts +++ b/src/renderer/src/components/editor/useMarkdownDocuments.ts @@ -1,5 +1,10 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { toast } from 'sonner' import type { MarkdownDocument } from '../../../../shared/types' +import { + isMarkdownDocumentListingCapacityError, + MARKDOWN_DOCUMENT_LISTING_ERROR_MESSAGE +} from '../../../../shared/markdown-document-listing-limits' import { useAppStore } from '@/store' import { getConnectionId } from '@/lib/connection-context' import { statRuntimePath } from '@/runtime/runtime-file-client' @@ -12,6 +17,10 @@ import { } from './markdown-doc-links' import { selectMarkdownDocumentWorktreePath } from './markdown-document-worktree-path-selector' import { requestSharedMarkdownDocumentList } from './markdown-document-list-request' +import { + retainMarkdownDocumentWorktreeSnapshot, + type MarkdownDocumentWorktreeSnapshot +} from './markdown-document-worktree-retention' type OpenMarkdownDocumentOptions = { anchor?: string | null @@ -60,8 +69,8 @@ export function useMarkdownDocuments( const openFile = useAppStore((s) => s.openFile) const openMarkdownPreview = useAppStore((s) => s.openMarkdownPreview) const [markdownDocumentsByWorktree, setMarkdownDocumentsByWorktree] = useState< - Record - >({}) + Map + >(() => new Map()) const requestRef = useRef(0) const connectionId = getConnectionId(worktreeId) @@ -91,17 +100,20 @@ export function useMarkdownDocuments( if (requestRef.current !== requestId) { return } - setMarkdownDocumentsByWorktree((prev) => ({ - ...prev, - [worktreeId]: documents - })) + setMarkdownDocumentsByWorktree((prev) => + retainMarkdownDocumentWorktreeSnapshot(prev, worktreeId, documents) + ) } catch (err) { console.error('Failed to list markdown documents:', err) if (requestRef.current === requestId) { - setMarkdownDocumentsByWorktree((prev) => ({ - ...prev, - [worktreeId]: [] - })) + if (isMarkdownDocumentListingCapacityError(err)) { + toast.error(MARKDOWN_DOCUMENT_LISTING_ERROR_MESSAGE, { + id: `markdown-document-listing-capacity:${worktreeId}` + }) + } + setMarkdownDocumentsByWorktree((prev) => + retainMarkdownDocumentWorktreeSnapshot(prev, worktreeId, []) + ) } } }, @@ -182,7 +194,7 @@ export function useMarkdownDocuments( }, [activeFile.id, isMarkdown, viewMode, refreshMarkdownDocuments]) const markdownDocuments = useMemo( - () => (worktreeId ? (markdownDocumentsByWorktree[worktreeId] ?? []) : []), + () => (worktreeId ? (markdownDocumentsByWorktree.get(worktreeId)?.documents ?? []) : []), [worktreeId, markdownDocumentsByWorktree] ) diff --git a/src/shared/markdown-document-listing-limits.test.ts b/src/shared/markdown-document-listing-limits.test.ts index 574e146b4ee..07178d2d13c 100644 --- a/src/shared/markdown-document-listing-limits.test.ts +++ b/src/shared/markdown-document-listing-limits.test.ts @@ -6,6 +6,11 @@ import { isMarkdownDocumentListingCapacityError, MARKDOWN_DOCUMENT_LISTING_ERROR_CODE, MARKDOWN_DOCUMENT_LISTING_ERROR_MESSAGE, + MARKDOWN_DOCUMENT_LISTING_MAX_DEPTH, + MARKDOWN_DOCUMENT_LISTING_MAX_DOCUMENTS, + MARKDOWN_DOCUMENT_LISTING_MAX_METADATA_BYTES, + MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES, + MARKDOWN_DOCUMENT_LISTING_MAX_VISITED_ENTRIES, MarkdownDocumentListingCapacityError, retainMarkdownDocument, visitMarkdownDocumentListingEntry @@ -68,6 +73,64 @@ describe('Markdown document listing limits', () => { ) }) + it('applies the default visited-entry ceiling of 100,000 when none is requested', () => { + // Why: the cases above pass explicit small limits, which only exercise clamping. Seeding + // relative to the constant would also pass at any constant value, so use the literal ceiling — + // raising the module constant has to fail here. + const belowCeiling = createMarkdownDocumentListingBudget() + belowCeiling.visitedEntries = 99_998 + visitMarkdownDocumentListingEntry(belowCeiling, 'last.md', 1) + expect(belowCeiling.visitedEntries).toBe(99_999) + + const atCeiling = createMarkdownDocumentListingBudget() + atCeiling.visitedEntries = 100_000 + expect(() => visitMarkdownDocumentListingEntry(atCeiling, 'overflow.md', 1)).toThrow( + MarkdownDocumentListingCapacityError + ) + }) + + it('applies the default metadata ceiling of 8 MiB when none is requested', () => { + const belowCeiling = createMarkdownDocumentListingBudget() + belowCeiling.metadataBytes = 8 * 1024 * 1024 - 4_096 + retainMarkdownDocument(belowCeiling, document('fits.md')) + + const atCeiling = createMarkdownDocumentListingBudget() + atCeiling.metadataBytes = 8 * 1024 * 1024 + expect(() => retainMarkdownDocument(atCeiling, document('overflow.md'))).toThrow( + MarkdownDocumentListingCapacityError + ) + }) + + it('applies the default depth ceiling of 256 when none is requested', () => { + const budget = createMarkdownDocumentListingBudget() + const path = `${'nested/'.repeat(256)}deep.md` + + visitMarkdownDocumentListingEntry(budget, path, 256) + expect(() => visitMarkdownDocumentListingEntry(budget, path, 257)).toThrow( + MarkdownDocumentListingCapacityError + ) + }) + + it('clamps a caller request above the module ceiling rather than honoring it', () => { + // Why: limits arrive from the renderer and the relay. A caller asking for a larger budget than + // the module allows must be clamped down, or the ceiling is advisory rather than enforced. + const budget = createMarkdownDocumentListingBudget({ + maxDocuments: Number.MAX_SAFE_INTEGER, + maxMetadataBytes: Number.MAX_SAFE_INTEGER, + maxPathBytes: Number.MAX_SAFE_INTEGER, + maxVisitedEntries: Number.MAX_SAFE_INTEGER, + maxDepth: Number.MAX_SAFE_INTEGER + }) + + expect(budget.limits).toEqual({ + maxDocuments: MARKDOWN_DOCUMENT_LISTING_MAX_DOCUMENTS, + maxMetadataBytes: MARKDOWN_DOCUMENT_LISTING_MAX_METADATA_BYTES, + maxPathBytes: MARKDOWN_DOCUMENT_LISTING_MAX_PATH_BYTES, + maxVisitedEntries: MARKDOWN_DOCUMENT_LISTING_MAX_VISITED_ENTRIES, + maxDepth: MARKDOWN_DOCUMENT_LISTING_MAX_DEPTH + }) + }) + it('recognizes structured runtime and Electron-wrapped capacity failures', () => { const structured = Object.assign(new Error('remote listing rejected'), { code: MARKDOWN_DOCUMENT_LISTING_ERROR_CODE