From 170fd31fc5177a4e8e608dceea97469817ffac16 Mon Sep 17 00:00:00 2001 From: Erik van der Boom Date: Wed, 22 Jul 2026 00:09:10 +0200 Subject: [PATCH 1/7] fix(review): enable marker region retrieval from committed chapter files --- mcp-ts/src/tools.ts | 43 ++++++++++++++++++++++++------ mcp-ts/test/review-markers.test.ts | 18 +++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/mcp-ts/src/tools.ts b/mcp-ts/src/tools.ts index 113593c..c475326 100644 --- a/mcp-ts/src/tools.ts +++ b/mcp-ts/src/tools.ts @@ -1643,13 +1643,16 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string const diffSection = filteredDiff.length > 0 ? formatReviewFiles(filteredDiff) : ''; const reviewedFiles = filteredDiff.map(f => f.file); - // ── 2. Marker regions in the same unstaged chapter files ──────────── - const markerFiles = collectReviewMarkerFiles(root, reviewedFiles); + // ── 2. Marker regions in story markdown files ──────────────────────── + const markerFiles = collectReviewMarkerFiles(root, language); const markerSection = markerFiles.length > 0 ? formatReviewMarkerFiles(markerFiles) : ''; // ── 3. Compose response ──────────────────────────────────────────────── + if (!gitAvailable && !diffSection) { + return 'Failed to run git diff. Is this a git repository?'; + } + if (!diffSection && !markerSection) { - if (!gitAvailable) { return 'Failed to run git diff. Is this a git repository?'; } return language === 'ALL' ? 'No uncommitted changes.' : `No uncommitted changes in ${language} files.`; @@ -1677,7 +1680,7 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string } // Strip marker lines so the next review pass is clean. Marker // consumption doesn't require git — the file edits are local. - if (markerFiles.length > 0) { + if (gitAvailable && markerFiles.length > 0) { consumeReviewMarkers(root, markerFiles.map(f => f.file)); } } @@ -1693,11 +1696,11 @@ function filterByLanguage(items: T[], language: stri }); } -/** Scan only the reviewed (unstaged chapter diff) files for review markers. */ -function collectReviewMarkerFiles(root: string, relFiles: string[]): FormattedMarkerFile[] { +/** 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 uniquePaths(relFiles)) { - if (!rel.toLowerCase().endsWith('.md')) { continue; } + for (const rel of listStoryMarkdownFiles(root)) { + if (!filterByLanguage([{ file: rel }], language).length) { continue; } const abs = path.join(root, rel); let content: string; try { content = fs.readFileSync(abs, 'utf-8'); } @@ -1714,6 +1717,30 @@ function chapterMarkdownPathspec(root: string): string { return `:(glob)${story}/**/*.md`; } +function listStoryMarkdownFiles(root: string): string[] { + const storyRoot = path.join(root, storyFolder(root)); + if (!fs.existsSync(storyRoot) || !fs.statSync(storyRoot).isDirectory()) { + return []; + } + + const files: string[] = []; + collectMarkdownFiles(storyRoot, files); + return uniquePaths(files.map(file => path.relative(root, file))); +} + +function collectMarkdownFiles(dir: string, acc: string[]): void { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + collectMarkdownFiles(fullPath, acc); + continue; + } + if (entry.isFile() && entry.name.toLowerCase().endsWith('.md')) { + acc.push(fullPath); + } + } +} + function uniquePaths(items: string[]): string[] { const seen = new Set(); const out: string[] = []; diff --git a/mcp-ts/test/review-markers.test.ts b/mcp-ts/test/review-markers.test.ts index 16dff66..8dc6f22 100644 --- a/mcp-ts/test/review-markers.test.ts +++ b/mcp-ts/test/review-markers.test.ts @@ -125,6 +125,24 @@ afterEach(() => { }); describe('toolGetReviewText — review markers', () => { + it('returns marker regions from committed chapter files without unstaged diffs', () => { + const root = makeGitRepo(); + const file = path.join(root, 'Story', 'NL', 'Chapter 1.md'); + write(file, + `# Hoofdstuk\n` + + `Vaste regel.\n` + + `${REVIEW_START_MARKER}\n` + + `Controleer deze alinea.\n` + + `${REVIEW_STOP_MARKER}\n`); + spawnSync('git', ['add', '.'], { cwd: root }); + spawnSync('git', ['commit', '-m', 'add committed marker region'], { cwd: root }); + + const out = toolGetReviewText(root, { language: 'NL' }); + expect(out).not.toContain('# Git diff'); + expect(out).toContain('# Review markers'); + expect(out).toContain('Controleer deze alinea.'); + }); + it('returns marker regions from unstaged chapter files', () => { const root = makeGitRepo(); const file = path.join(root, 'Story', 'EN', 'Chapter 1.md'); From 337fa875924da088fb486de60c76e6aa05dd3beb Mon Sep 17 00:00:00 2001 From: Erik van der Boom Date: Wed, 22 Jul 2026 00:13:31 +0200 Subject: [PATCH 2/7] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- mcp-ts/src/tools.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mcp-ts/src/tools.ts b/mcp-ts/src/tools.ts index c475326..cf1e1ea 100644 --- a/mcp-ts/src/tools.ts +++ b/mcp-ts/src/tools.ts @@ -1729,7 +1729,13 @@ function listStoryMarkdownFiles(root: string): string[] { } function collectMarkdownFiles(dir: string, acc: string[]): void { - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) { collectMarkdownFiles(fullPath, acc); 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 3/7] 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'); From 83ea70b9d4a54ddb61db806dbef3f79b055a3de9 Mon Sep 17 00:00:00 2001 From: Erik van der Boom Date: Wed, 22 Jul 2026 00:21:21 +0200 Subject: [PATCH 4/7] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- mcp-ts/src/tools.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mcp-ts/src/tools.ts b/mcp-ts/src/tools.ts index 6ecc206..8248ece 100644 --- a/mcp-ts/src/tools.ts +++ b/mcp-ts/src/tools.ts @@ -1734,7 +1734,9 @@ function fileMatchesLanguage(root: string, file: string, language: string): bool function getStoryScanRoots(root: string, language: string): string[] { const storyRoot = path.join(root, storyFolder(root)); - if (!fs.existsSync(storyRoot) || !fs.statSync(storyRoot).isDirectory()) { + try { + if (!fs.statSync(storyRoot).isDirectory()) { return []; } + } catch { return []; } if (language === 'ALL') { From 40394d52fee911926690a4930aea63b09464d0e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Jul 2026 22:25:40 +0000 Subject: [PATCH 5/7] fix: avoid marker scan when git diff is unavailable --- mcp-ts/src/tools.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/mcp-ts/src/tools.ts b/mcp-ts/src/tools.ts index 8248ece..be7387c 100644 --- a/mcp-ts/src/tools.ts +++ b/mcp-ts/src/tools.ts @@ -1643,15 +1643,15 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string const diffSection = filteredDiff.length > 0 ? formatReviewFiles(filteredDiff) : ''; const reviewedFiles = filteredDiff.map(f => f.file); + if (!gitAvailable) { + return 'Failed to run git diff. Is this a git repository?'; + } + // ── 2. Marker regions in story markdown files ──────────────────────── const markerFiles = collectReviewMarkerFiles(root, language); const markerSection = markerFiles.length > 0 ? formatReviewMarkerFiles(markerFiles) : ''; // ── 3. Compose response ──────────────────────────────────────────────── - if (!gitAvailable && !diffSection) { - return 'Failed to run git diff. Is this a git repository?'; - } - if (!diffSection && !markerSection) { return language === 'ALL' ? 'No uncommitted changes.' @@ -1659,9 +1659,7 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string } const parts: string[] = []; - if (!gitAvailable) { - parts.push('# Git diff unavailable', 'Failed to run git diff. Is this a git repository?'); - } else if (diffSection) { + if (diffSection) { parts.push('# Git diff', diffSection); } if (markerSection) { parts.push('# Review markers', markerSection); } @@ -1678,9 +1676,9 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string } catch { /* best effort — staging failure shouldn't break the review */ } } } - // Strip marker lines so the next review pass is clean. Marker - // consumption doesn't require git — the file edits are local. - if (gitAvailable && markerFiles.length > 0) { + // Strip marker lines so the next review pass is clean. This performs + // local file edits and then best-effort git add on touched files. + if (markerFiles.length > 0) { consumeReviewMarkers(root, markerFiles.map(f => f.file)); } } From 5310e631aeec063d5fb90954a18473170387a791 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Jul 2026 22:26:43 +0000 Subject: [PATCH 6/7] refactor: short-circuit before diff parsing when git unavailable --- mcp-ts/src/tools.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mcp-ts/src/tools.ts b/mcp-ts/src/tools.ts index be7387c..7c23e3a 100644 --- a/mcp-ts/src/tools.ts +++ b/mcp-ts/src/tools.ts @@ -1638,15 +1638,15 @@ export function toolGetReviewText(root: string, args: GetReviewTextArgs): string gitAvailable = false; } + if (!gitAvailable) { + return 'Failed to run git diff. Is this a git repository?'; + } + const diffFiles = raw.trim() ? parseUnifiedDiff(raw) : []; const filteredDiff = filterByLanguage(root, diffFiles, language); const diffSection = filteredDiff.length > 0 ? formatReviewFiles(filteredDiff) : ''; const reviewedFiles = filteredDiff.map(f => f.file); - if (!gitAvailable) { - return 'Failed to run git diff. Is this a git repository?'; - } - // ── 2. Marker regions in story markdown files ──────────────────────── const markerFiles = collectReviewMarkerFiles(root, language); const markerSection = markerFiles.length > 0 ? formatReviewMarkerFiles(markerFiles) : ''; From 2401d382caf5e7888cd1aba196a929620948e049 Mon Sep 17 00:00:00 2001 From: Erik van der Boom Date: Wed, 22 Jul 2026 00:36:12 +0200 Subject: [PATCH 7/7] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- mcp-ts/src/tools.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mcp-ts/src/tools.ts b/mcp-ts/src/tools.ts index 7c23e3a..f56f785 100644 --- a/mcp-ts/src/tools.ts +++ b/mcp-ts/src/tools.ts @@ -1758,7 +1758,8 @@ 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 ?? []) { + const languages = Array.isArray(settings?.languages) ? settings.languages : []; + for (const entry of languages) { if (entry.code.toUpperCase() !== upper) { continue; } if (typeof entry.folderName === 'string' && entry.folderName.trim()) { names.add(entry.folderName.trim());