From e171482f9a869b9896cfedcffa24e5edc37addf8 Mon Sep 17 00:00:00 2001 From: Erik van der Boom Date: Wed, 22 Jul 2026 00:16:55 +0200 Subject: [PATCH] review comment fix for traversal --- mcp-ts/src/tools.ts | 63 ++++++++++++++++++++++++------ mcp-ts/test/review-markers.test.ts | 21 ++++++++++ 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/mcp-ts/src/tools.ts b/mcp-ts/src/tools.ts index cf1e1ea..6ecc206 100644 --- a/mcp-ts/src/tools.ts +++ b/mcp-ts/src/tools.ts @@ -1639,7 +1639,7 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string } const diffFiles = raw.trim() ? parseUnifiedDiff(raw) : []; - const filteredDiff = filterByLanguage(diffFiles, language); + const filteredDiff = filterByLanguage(root, diffFiles, language); const diffSection = filteredDiff.length > 0 ? formatReviewFiles(filteredDiff) : ''; const reviewedFiles = filteredDiff.map(f => f.file); @@ -1688,19 +1688,15 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string return result; } -function filterByLanguage(items: T[], language: string): T[] { +function filterByLanguage(root: string, items: T[], language: string): T[] { if (language === 'ALL') { return items; } - return items.filter(f => { - const upper = f.file.toUpperCase().replaceAll('\\', '/'); - return upper.includes(`/${language}/`); - }); + return items.filter(f => fileMatchesLanguage(root, f.file, language)); } /** Scan story markdown files for review markers, even when the file is already committed. */ function collectReviewMarkerFiles(root: string, language: string): FormattedMarkerFile[] { const out: FormattedMarkerFile[] = []; - for (const rel of listStoryMarkdownFiles(root)) { - if (!filterByLanguage([{ file: rel }], language).length) { continue; } + for (const rel of listStoryMarkdownFiles(root, language)) { const abs = path.join(root, rel); let content: string; try { content = fs.readFileSync(abs, 'utf-8'); } @@ -1717,15 +1713,58 @@ function chapterMarkdownPathspec(root: string): string { return `:(glob)${story}/**/*.md`; } -function listStoryMarkdownFiles(root: string): string[] { +function listStoryMarkdownFiles(root: string, language: string): string[] { + const storyRoots = getStoryScanRoots(root, language); + const files: string[] = []; + for (const storyRoot of storyRoots) { + collectMarkdownFiles(storyRoot, files); + } + return uniquePaths(files.map(file => path.relative(root, file))); +} + +function fileMatchesLanguage(root: string, file: string, language: string): boolean { + if (language === 'ALL') { return true; } + const normalizedFile = file.replaceAll('\\', '/').toUpperCase(); + const story = storyFolder(root).replaceAll('\\', '/').replace(/^\/+|\/+$/g, '').toUpperCase(); + return getLanguageFolderNames(root, language).some(folder => { + const prefix = `${story}/${folder.toUpperCase()}/`; + return normalizedFile.startsWith(prefix) || normalizedFile.includes(`/${prefix}`); + }); +} + +function getStoryScanRoots(root: string, language: string): string[] { const storyRoot = path.join(root, storyFolder(root)); if (!fs.existsSync(storyRoot) || !fs.statSync(storyRoot).isDirectory()) { return []; } + if (language === 'ALL') { + return [storyRoot]; + } - const files: string[] = []; - collectMarkdownFiles(storyRoot, files); - return uniquePaths(files.map(file => path.relative(root, file))); + const roots = getLanguageFolderNames(root, language) + .map(folder => path.join(storyRoot, folder)) + .filter(dir => { + try { + return fs.existsSync(dir) && fs.statSync(dir).isDirectory(); + } catch { + return false; + } + }); + + return roots.length > 0 ? uniquePaths(roots) : []; +} + +function getLanguageFolderNames(root: string, language: string): string[] { + const upper = language.toUpperCase(); + const names = new Set([upper]); + const settings = readSettings(root); + for (const entry of settings?.languages ?? []) { + if (entry.code.toUpperCase() !== upper) { continue; } + if (typeof entry.folderName === 'string' && entry.folderName.trim()) { + names.add(entry.folderName.trim()); + } + } + return Array.from(names); } function collectMarkdownFiles(dir: string, acc: string[]): void { diff --git a/mcp-ts/test/review-markers.test.ts b/mcp-ts/test/review-markers.test.ts index 8dc6f22..47ea2af 100644 --- a/mcp-ts/test/review-markers.test.ts +++ b/mcp-ts/test/review-markers.test.ts @@ -227,6 +227,27 @@ describe('toolGetReviewText — review markers', () => { expect(out).not.toContain('EN region'); }); + it('language filter uses configured folder names for marker scans', () => { + const root = makeGitRepo(); + write(path.join(root, '.bindery', 'settings.json'), JSON.stringify({ + storyFolder: 'Story', + languages: [ + { code: 'EN', folderName: 'English', chapterWord: 'Chapter', actPrefix: 'Act', prologueLabel: 'Prologue', epilogueLabel: 'Epilogue' }, + { code: 'NL', folderName: 'Dutch', chapterWord: 'Hoofdstuk', actPrefix: 'Akte', prologueLabel: 'Proloog', epilogueLabel: 'Epiloog' }, + ], + })); + write(path.join(root, 'Story', 'English', 'Chapter 1.md'), + `${REVIEW_START_MARKER}\nEN region\n${REVIEW_STOP_MARKER}\n`); + write(path.join(root, 'Story', 'Dutch', 'Chapter 1.md'), + `${REVIEW_START_MARKER}\nNL region\n${REVIEW_STOP_MARKER}\n`); + spawnSync('git', ['add', '.'], { cwd: root }); + spawnSync('git', ['commit', '-m', 'add custom language folders'], { cwd: root }); + + const out = toolGetReviewText(root, { language: 'NL' }); + expect(out).toContain('NL region'); + expect(out).not.toContain('EN region'); + }); + it('reports an unclosed marker as open-ended in the output', () => { const root = makeGitRepo(); const file = path.join(root, 'Story', 'EN', 'Chapter 1.md');