From f13b5fa788a503df87867caf523ce8ab8561a323 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Mon, 2 Nov 2026 21:41:57 -0600 Subject: [PATCH 1/4] fix: bound table rendering, glob expansion and e2e server reuse - Render markdown tables larger than 2,000 cells (rows x columns) in the document view instead of the comparison table. - analyze_sources.py: expand glob patterns with an iterative walk that does not follow directory symlinks under ** and stops after 10,000 matches. Applied to all three vendored copies. - Playwright: reuse an existing dev server only outside CI. Fixes #557 Fixes #558 Co-Authored-By: Claude Opus 5.5 --- .../scripts/analyze_sources.py | 86 ++++++++++++++++++- .../scripts/tests/test_analyze_sources.py | 49 +++++++++++ .../scripts/analyze_sources.py | 86 ++++++++++++++++++- .../scripts/tests/test_analyze_sources.py | 49 +++++++++++ .../scripts/analyze_sources.py | 86 ++++++++++++++++++- .../scripts/tests/test_analyze_sources.py | 49 +++++++++++ playwright.config.ts | 3 +- playwright.electron.config.ts | 3 +- .../types/domain/render-markdown.test.ts | 22 +++++ src/shared/types/domain/render-markdown.ts | 15 +++- 10 files changed, 432 insertions(+), 16 deletions(-) diff --git a/.claude/skills/bmad-distillator/scripts/analyze_sources.py b/.claude/skills/bmad-distillator/scripts/analyze_sources.py index 38ddcbe3..aa53a80f 100644 --- a/.claude/skills/bmad-distillator/scripts/analyze_sources.py +++ b/.claude/skills/bmad-distillator/scripts/analyze_sources.py @@ -10,6 +10,8 @@ Accepts: file paths, folder paths (scans recursively for .md/.txt/.yaml/.yml/.json), or glob patterns. Skips node_modules, .git, __pycache__, .venv, _bmad-output. +Glob patterns do not follow directory symlinks under `**` and stop after +MAX_GLOB_MATCHES paths. Output JSON structure: status: "ok" | "error" @@ -27,7 +29,7 @@ from __future__ import annotations import argparse -import glob +import fnmatch import json import os import re @@ -46,6 +48,11 @@ # Approximate chars per token for estimation CHARS_PER_TOKEN = 4 +# Most paths a single glob pattern may match before the rest are dropped +MAX_GLOB_MATCHES = 10_000 + +GLOB_MAGIC = re.compile(r"[*?[]") + # Thresholds SINGLE_COMPRESSOR_MAX_TOKENS = 15_000 SINGLE_DISTILLATE_MAX_TOKENS = 5_000 @@ -79,6 +86,79 @@ ] +def _scandir(directory: Path) -> list[os.DirEntry]: + try: + with os.scandir(directory) as it: + return sorted(it, key=lambda e: e.name) + except OSError: + return [] + + +def iter_glob(pattern: str, limit: int = MAX_GLOB_MATCHES) -> list[Path]: + """Expand a glob pattern like glob.glob(pattern, recursive=True), but iteratively, + without following directory symlinks under `**`, and returning at most `limit` paths. + + As with glob, `*` and `?` stay within one path segment, `**` matches zero or more + directories, and hidden names only match a segment that starts with a dot. + """ + parts = Path(pattern).parts + first_magic = next((i for i, p in enumerate(parts) if GLOB_MAGIC.search(p)), len(parts)) + base = Path(*parts[:first_magic]) if first_magic else Path(".") + segments = parts[first_magic:] + if not segments: + return [base] if base.exists() else [] + if not base.is_dir(): + return [] + + matches: list[Path] = [] + seen: set[Path] = set() + visited: set[tuple[Path, int]] = set() + stack: list[tuple[Path, int]] = [(base, 0)] + + def add(path: Path) -> bool: + if path not in seen: + seen.add(path) + matches.append(path) + return len(matches) >= limit + + while stack: + current, index = stack.pop() + if (current, index) in visited: + continue + visited.add((current, index)) + if index == len(segments): + if add(current): + break + continue + segment = segments[index] + last = index == len(segments) - 1 + if segment == "**": + stack.append((current, index + 1)) + for entry in reversed(_scandir(current)): + if entry.name.startswith("."): + continue + if entry.is_dir(follow_symlinks=False): + stack.append((Path(entry.path), index)) + elif last and add(Path(entry.path)): + stack.clear() + break + continue + for entry in reversed(_scandir(current)): + if entry.name.startswith(".") and not segment.startswith("."): + continue + if not fnmatch.fnmatch(entry.name, segment): + continue + if last or entry.is_dir(): + stack.append((Path(entry.path), index + 1)) + + if len(matches) >= limit: + print( + f"warning: {pattern!r} matched {limit:,}+ paths; ignoring the rest", + file=sys.stderr, + ) + return matches + + def resolve_inputs(inputs: list[str]) -> list[Path]: """Resolve input arguments to a flat list of file paths.""" files: list[Path] = [] @@ -95,9 +175,7 @@ def resolve_inputs(inputs: list[str]) -> list[Path]: files.append(fp.resolve()) else: # Try as glob - matches = glob.glob(inp, recursive=True) - for m in sorted(matches): - mp = Path(m) + for mp in sorted(iter_glob(inp)): if mp.is_file() and mp.suffix.lower() in INCLUDE_EXTENSIONS: files.append(mp.resolve()) # Deduplicate while preserving order diff --git a/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py b/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py index 3c65ef20..e0974deb 100644 --- a/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py +++ b/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py @@ -13,6 +13,7 @@ sys.path.insert(0, str(Path(__file__).parent.parent)) from analyze_sources import ( + iter_glob, resolve_inputs, detect_doc_type, suggest_groups, @@ -94,6 +95,54 @@ def test_nonexistent_path(self): assert len(result) == 0 +class TestIterGlob: + @pytest.mark.parametrize("pattern", [ + "*.md", + "**/*.md", + "sub*/*.md", + "**", + "subdir/**/*.md", + "*/prd-?2.md", + "[pr]*.md", + "missing/**/*.md", + ]) + def test_matches_glob_on_plain_trees(self, temp_dir, pattern): + import glob + + full = str(Path(temp_dir) / pattern) + expected = {Path(p) for p in glob.glob(full, recursive=True)} + assert set(iter_glob(full)) == expected + + def test_skips_hidden_names_unless_the_segment_starts_with_a_dot(self, temp_dir): + hidden = Path(temp_dir) / ".hidden" + hidden.mkdir() + (hidden / "secret.md").write_text("x") + assert hidden / "secret.md" not in iter_glob(str(Path(temp_dir) / "**" / "*.md")) + assert hidden / "secret.md" in iter_glob(str(Path(temp_dir) / ".hidden" / "*.md")) + + def test_does_not_follow_directory_symlinks_under_double_star(self, temp_dir): + loop = Path(temp_dir) / "subdir" / "loop" + loop.symlink_to(Path(temp_dir), target_is_directory=True) + (Path(temp_dir) / "subdir" / "loop2").symlink_to( + Path(temp_dir), target_is_directory=True + ) + result = iter_glob(str(Path(temp_dir) / "**" / "*.md")) + assert not any("loop" in p.parts for p in result) + assert Path(temp_dir) / "subdir" / "prd-v2.md" in result + + def test_stops_at_the_limit(self, temp_dir, capsys): + for i in range(5): + (Path(temp_dir) / f"extra-{i}.md").write_text("x") + result = iter_glob(str(Path(temp_dir) / "*.md"), limit=3) + assert len(result) == 3 + assert "matched 3+ paths" in capsys.readouterr().err + + def test_returns_a_literal_path_only_if_it_exists(self, temp_dir): + existing = str(Path(temp_dir) / "random.txt") + assert iter_glob(existing) == [Path(existing)] + assert iter_glob(str(Path(temp_dir) / "nope.txt")) == [] + + class TestDetectDocType: @pytest.mark.parametrize("filename,expected", [ ("product-brief-foo.md", "product-brief"), diff --git a/_bmad/core/bmad-distillator/scripts/analyze_sources.py b/_bmad/core/bmad-distillator/scripts/analyze_sources.py index 38ddcbe3..aa53a80f 100644 --- a/_bmad/core/bmad-distillator/scripts/analyze_sources.py +++ b/_bmad/core/bmad-distillator/scripts/analyze_sources.py @@ -10,6 +10,8 @@ Accepts: file paths, folder paths (scans recursively for .md/.txt/.yaml/.yml/.json), or glob patterns. Skips node_modules, .git, __pycache__, .venv, _bmad-output. +Glob patterns do not follow directory symlinks under `**` and stop after +MAX_GLOB_MATCHES paths. Output JSON structure: status: "ok" | "error" @@ -27,7 +29,7 @@ from __future__ import annotations import argparse -import glob +import fnmatch import json import os import re @@ -46,6 +48,11 @@ # Approximate chars per token for estimation CHARS_PER_TOKEN = 4 +# Most paths a single glob pattern may match before the rest are dropped +MAX_GLOB_MATCHES = 10_000 + +GLOB_MAGIC = re.compile(r"[*?[]") + # Thresholds SINGLE_COMPRESSOR_MAX_TOKENS = 15_000 SINGLE_DISTILLATE_MAX_TOKENS = 5_000 @@ -79,6 +86,79 @@ ] +def _scandir(directory: Path) -> list[os.DirEntry]: + try: + with os.scandir(directory) as it: + return sorted(it, key=lambda e: e.name) + except OSError: + return [] + + +def iter_glob(pattern: str, limit: int = MAX_GLOB_MATCHES) -> list[Path]: + """Expand a glob pattern like glob.glob(pattern, recursive=True), but iteratively, + without following directory symlinks under `**`, and returning at most `limit` paths. + + As with glob, `*` and `?` stay within one path segment, `**` matches zero or more + directories, and hidden names only match a segment that starts with a dot. + """ + parts = Path(pattern).parts + first_magic = next((i for i, p in enumerate(parts) if GLOB_MAGIC.search(p)), len(parts)) + base = Path(*parts[:first_magic]) if first_magic else Path(".") + segments = parts[first_magic:] + if not segments: + return [base] if base.exists() else [] + if not base.is_dir(): + return [] + + matches: list[Path] = [] + seen: set[Path] = set() + visited: set[tuple[Path, int]] = set() + stack: list[tuple[Path, int]] = [(base, 0)] + + def add(path: Path) -> bool: + if path not in seen: + seen.add(path) + matches.append(path) + return len(matches) >= limit + + while stack: + current, index = stack.pop() + if (current, index) in visited: + continue + visited.add((current, index)) + if index == len(segments): + if add(current): + break + continue + segment = segments[index] + last = index == len(segments) - 1 + if segment == "**": + stack.append((current, index + 1)) + for entry in reversed(_scandir(current)): + if entry.name.startswith("."): + continue + if entry.is_dir(follow_symlinks=False): + stack.append((Path(entry.path), index)) + elif last and add(Path(entry.path)): + stack.clear() + break + continue + for entry in reversed(_scandir(current)): + if entry.name.startswith(".") and not segment.startswith("."): + continue + if not fnmatch.fnmatch(entry.name, segment): + continue + if last or entry.is_dir(): + stack.append((Path(entry.path), index + 1)) + + if len(matches) >= limit: + print( + f"warning: {pattern!r} matched {limit:,}+ paths; ignoring the rest", + file=sys.stderr, + ) + return matches + + def resolve_inputs(inputs: list[str]) -> list[Path]: """Resolve input arguments to a flat list of file paths.""" files: list[Path] = [] @@ -95,9 +175,7 @@ def resolve_inputs(inputs: list[str]) -> list[Path]: files.append(fp.resolve()) else: # Try as glob - matches = glob.glob(inp, recursive=True) - for m in sorted(matches): - mp = Path(m) + for mp in sorted(iter_glob(inp)): if mp.is_file() and mp.suffix.lower() in INCLUDE_EXTENSIONS: files.append(mp.resolve()) # Deduplicate while preserving order diff --git a/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py b/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py index 3c65ef20..e0974deb 100644 --- a/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py +++ b/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py @@ -13,6 +13,7 @@ sys.path.insert(0, str(Path(__file__).parent.parent)) from analyze_sources import ( + iter_glob, resolve_inputs, detect_doc_type, suggest_groups, @@ -94,6 +95,54 @@ def test_nonexistent_path(self): assert len(result) == 0 +class TestIterGlob: + @pytest.mark.parametrize("pattern", [ + "*.md", + "**/*.md", + "sub*/*.md", + "**", + "subdir/**/*.md", + "*/prd-?2.md", + "[pr]*.md", + "missing/**/*.md", + ]) + def test_matches_glob_on_plain_trees(self, temp_dir, pattern): + import glob + + full = str(Path(temp_dir) / pattern) + expected = {Path(p) for p in glob.glob(full, recursive=True)} + assert set(iter_glob(full)) == expected + + def test_skips_hidden_names_unless_the_segment_starts_with_a_dot(self, temp_dir): + hidden = Path(temp_dir) / ".hidden" + hidden.mkdir() + (hidden / "secret.md").write_text("x") + assert hidden / "secret.md" not in iter_glob(str(Path(temp_dir) / "**" / "*.md")) + assert hidden / "secret.md" in iter_glob(str(Path(temp_dir) / ".hidden" / "*.md")) + + def test_does_not_follow_directory_symlinks_under_double_star(self, temp_dir): + loop = Path(temp_dir) / "subdir" / "loop" + loop.symlink_to(Path(temp_dir), target_is_directory=True) + (Path(temp_dir) / "subdir" / "loop2").symlink_to( + Path(temp_dir), target_is_directory=True + ) + result = iter_glob(str(Path(temp_dir) / "**" / "*.md")) + assert not any("loop" in p.parts for p in result) + assert Path(temp_dir) / "subdir" / "prd-v2.md" in result + + def test_stops_at_the_limit(self, temp_dir, capsys): + for i in range(5): + (Path(temp_dir) / f"extra-{i}.md").write_text("x") + result = iter_glob(str(Path(temp_dir) / "*.md"), limit=3) + assert len(result) == 3 + assert "matched 3+ paths" in capsys.readouterr().err + + def test_returns_a_literal_path_only_if_it_exists(self, temp_dir): + existing = str(Path(temp_dir) / "random.txt") + assert iter_glob(existing) == [Path(existing)] + assert iter_glob(str(Path(temp_dir) / "nope.txt")) == [] + + class TestDetectDocType: @pytest.mark.parametrize("filename,expected", [ ("product-brief-foo.md", "product-brief"), diff --git a/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py b/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py index 38ddcbe3..aa53a80f 100644 --- a/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py +++ b/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py @@ -10,6 +10,8 @@ Accepts: file paths, folder paths (scans recursively for .md/.txt/.yaml/.yml/.json), or glob patterns. Skips node_modules, .git, __pycache__, .venv, _bmad-output. +Glob patterns do not follow directory symlinks under `**` and stop after +MAX_GLOB_MATCHES paths. Output JSON structure: status: "ok" | "error" @@ -27,7 +29,7 @@ from __future__ import annotations import argparse -import glob +import fnmatch import json import os import re @@ -46,6 +48,11 @@ # Approximate chars per token for estimation CHARS_PER_TOKEN = 4 +# Most paths a single glob pattern may match before the rest are dropped +MAX_GLOB_MATCHES = 10_000 + +GLOB_MAGIC = re.compile(r"[*?[]") + # Thresholds SINGLE_COMPRESSOR_MAX_TOKENS = 15_000 SINGLE_DISTILLATE_MAX_TOKENS = 5_000 @@ -79,6 +86,79 @@ ] +def _scandir(directory: Path) -> list[os.DirEntry]: + try: + with os.scandir(directory) as it: + return sorted(it, key=lambda e: e.name) + except OSError: + return [] + + +def iter_glob(pattern: str, limit: int = MAX_GLOB_MATCHES) -> list[Path]: + """Expand a glob pattern like glob.glob(pattern, recursive=True), but iteratively, + without following directory symlinks under `**`, and returning at most `limit` paths. + + As with glob, `*` and `?` stay within one path segment, `**` matches zero or more + directories, and hidden names only match a segment that starts with a dot. + """ + parts = Path(pattern).parts + first_magic = next((i for i, p in enumerate(parts) if GLOB_MAGIC.search(p)), len(parts)) + base = Path(*parts[:first_magic]) if first_magic else Path(".") + segments = parts[first_magic:] + if not segments: + return [base] if base.exists() else [] + if not base.is_dir(): + return [] + + matches: list[Path] = [] + seen: set[Path] = set() + visited: set[tuple[Path, int]] = set() + stack: list[tuple[Path, int]] = [(base, 0)] + + def add(path: Path) -> bool: + if path not in seen: + seen.add(path) + matches.append(path) + return len(matches) >= limit + + while stack: + current, index = stack.pop() + if (current, index) in visited: + continue + visited.add((current, index)) + if index == len(segments): + if add(current): + break + continue + segment = segments[index] + last = index == len(segments) - 1 + if segment == "**": + stack.append((current, index + 1)) + for entry in reversed(_scandir(current)): + if entry.name.startswith("."): + continue + if entry.is_dir(follow_symlinks=False): + stack.append((Path(entry.path), index)) + elif last and add(Path(entry.path)): + stack.clear() + break + continue + for entry in reversed(_scandir(current)): + if entry.name.startswith(".") and not segment.startswith("."): + continue + if not fnmatch.fnmatch(entry.name, segment): + continue + if last or entry.is_dir(): + stack.append((Path(entry.path), index + 1)) + + if len(matches) >= limit: + print( + f"warning: {pattern!r} matched {limit:,}+ paths; ignoring the rest", + file=sys.stderr, + ) + return matches + + def resolve_inputs(inputs: list[str]) -> list[Path]: """Resolve input arguments to a flat list of file paths.""" files: list[Path] = [] @@ -95,9 +175,7 @@ def resolve_inputs(inputs: list[str]) -> list[Path]: files.append(fp.resolve()) else: # Try as glob - matches = glob.glob(inp, recursive=True) - for m in sorted(matches): - mp = Path(m) + for mp in sorted(iter_glob(inp)): if mp.is_file() and mp.suffix.lower() in INCLUDE_EXTENSIONS: files.append(mp.resolve()) # Deduplicate while preserving order diff --git a/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py b/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py index 3c65ef20..e0974deb 100644 --- a/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py +++ b/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py @@ -13,6 +13,7 @@ sys.path.insert(0, str(Path(__file__).parent.parent)) from analyze_sources import ( + iter_glob, resolve_inputs, detect_doc_type, suggest_groups, @@ -94,6 +95,54 @@ def test_nonexistent_path(self): assert len(result) == 0 +class TestIterGlob: + @pytest.mark.parametrize("pattern", [ + "*.md", + "**/*.md", + "sub*/*.md", + "**", + "subdir/**/*.md", + "*/prd-?2.md", + "[pr]*.md", + "missing/**/*.md", + ]) + def test_matches_glob_on_plain_trees(self, temp_dir, pattern): + import glob + + full = str(Path(temp_dir) / pattern) + expected = {Path(p) for p in glob.glob(full, recursive=True)} + assert set(iter_glob(full)) == expected + + def test_skips_hidden_names_unless_the_segment_starts_with_a_dot(self, temp_dir): + hidden = Path(temp_dir) / ".hidden" + hidden.mkdir() + (hidden / "secret.md").write_text("x") + assert hidden / "secret.md" not in iter_glob(str(Path(temp_dir) / "**" / "*.md")) + assert hidden / "secret.md" in iter_glob(str(Path(temp_dir) / ".hidden" / "*.md")) + + def test_does_not_follow_directory_symlinks_under_double_star(self, temp_dir): + loop = Path(temp_dir) / "subdir" / "loop" + loop.symlink_to(Path(temp_dir), target_is_directory=True) + (Path(temp_dir) / "subdir" / "loop2").symlink_to( + Path(temp_dir), target_is_directory=True + ) + result = iter_glob(str(Path(temp_dir) / "**" / "*.md")) + assert not any("loop" in p.parts for p in result) + assert Path(temp_dir) / "subdir" / "prd-v2.md" in result + + def test_stops_at_the_limit(self, temp_dir, capsys): + for i in range(5): + (Path(temp_dir) / f"extra-{i}.md").write_text("x") + result = iter_glob(str(Path(temp_dir) / "*.md"), limit=3) + assert len(result) == 3 + assert "matched 3+ paths" in capsys.readouterr().err + + def test_returns_a_literal_path_only_if_it_exists(self, temp_dir): + existing = str(Path(temp_dir) / "random.txt") + assert iter_glob(existing) == [Path(existing)] + assert iter_glob(str(Path(temp_dir) / "nope.txt")) == [] + + class TestDetectDocType: @pytest.mark.parametrize("filename,expected", [ ("product-brief-foo.md", "product-brief"), diff --git a/playwright.config.ts b/playwright.config.ts index 25b4756b..6b15d4b6 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -18,7 +18,8 @@ export default defineConfig({ webServer: { command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', - reuseExistingServer: true, + // Locally, reuse a dev server already on the port; CI always starts its own. + reuseExistingServer: process.env['CI'] !== 'true', timeout: 30000, stdout: 'pipe', stderr: 'pipe', diff --git a/playwright.electron.config.ts b/playwright.electron.config.ts index cd6e8e6a..bd8eb889 100644 --- a/playwright.electron.config.ts +++ b/playwright.electron.config.ts @@ -24,7 +24,8 @@ export default defineConfig({ webServer: { command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', - reuseExistingServer: true, + // Locally, reuse a dev server already on the port; CI always starts its own. + reuseExistingServer: process.env['CI'] !== 'true', timeout: 30000, stdout: 'pipe', stderr: 'pipe', diff --git a/src/shared/types/domain/render-markdown.test.ts b/src/shared/types/domain/render-markdown.test.ts index 6a6152f2..6250177e 100644 --- a/src/shared/types/domain/render-markdown.test.ts +++ b/src/shared/types/domain/render-markdown.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { detectContentType, isDocumentContent, + MAX_TABLE_CELLS, parseMarkdownTable, parsePlanContent, parseClusteredContent, @@ -57,6 +58,27 @@ describe('detectContentType', () => { expect(detectContentType(tableWithWarning)).toBe('document'); }); + describe('table size cap', () => { + const table = (columns: number, rows: number): string => { + const header = `|${Array.from({ length: columns }, (_, i) => ` H${String(i)} `).join('|')}|`; + const separator = `|${Array.from({ length: columns }, () => '---').join('|')}|`; + // Each row has only the first cell; the rest would be filled in when rendered. + return [header, separator, ...Array.from({ length: rows }, () => '| x |')].join('\n'); + }; + + it('renders a table of exactly MAX_TABLE_CELLS cells as a comparison table', () => { + expect(detectContentType(table(2, MAX_TABLE_CELLS / 2))).toBe('comparison-table'); + }); + + it('falls back to the document view once rows x columns exceeds the cap', () => { + expect(detectContentType(table(2, MAX_TABLE_CELLS / 2 + 1))).toBe('document'); + }); + + it('falls back for a wide header with short rows', () => { + expect(detectContentType(table(MAX_TABLE_CELLS / 2, 3))).toBe('document'); + }); + }); + it('returns "plan" for numbered steps with checkboxes', () => { const plan = ['- [x] Set up project', '- [ ] Write tests', '- [ ] Deploy to production'].join( '\n', diff --git a/src/shared/types/domain/render-markdown.ts b/src/shared/types/domain/render-markdown.ts index 901d790b..33b55025 100644 --- a/src/shared/types/domain/render-markdown.ts +++ b/src/shared/types/domain/render-markdown.ts @@ -47,11 +47,20 @@ export function detectContentType(text: string): ContentType { return 'text'; } +/** + * Largest table (rows x columns) shown as a comparison table. Short rows are padded to the + * header width when rendered, so a few lines of text could otherwise expand without bound. + */ +export const MAX_TABLE_CELLS = 2_000; + const TABLE_ROW_LINE = /^\|.+\|$/; const CLUSTER_HEADER_LINE = /^\*\*[^*]+\*\*$/; const CLUSTER_ITEM_LINE = /^[-*][ \t]+\S/; -/** Check whether every non-empty line in `text` is part of the markdown table and lossless. */ +/** + * Check whether every non-empty line in `text` is part of the markdown table, the table is + * lossless, and it is small enough to render as a comparison table. + */ function isLosslessTable(text: string): boolean { const lines = text .split('\n') @@ -62,7 +71,9 @@ function isLosslessTable(text: string): boolean { } const parsed = parseMarkdownTable(text); return ( - parsed.headers.length >= 2 && parsed.rows.every((row) => row.length <= parsed.headers.length) + parsed.headers.length >= 2 && + parsed.headers.length * parsed.rows.length <= MAX_TABLE_CELLS && + parsed.rows.every((row) => row.length <= parsed.headers.length) ); } From 9b9974eb3b5b7586cb954328036d432dff0a8766 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 06:16:32 -0600 Subject: [PATCH 2/4] fix: match glob edge cases in analyze_sources and treat any CI value as CI - iter_glob: treat symlink loops and unreadable entries as non-directories instead of raising, honour a trailing separator (directories only), follow `.`/`..` segments after a wildcard, and match nothing for an empty pattern. Same in all three copies. - Playwright: any non-empty CI value (e.g. CI=1) disables server reuse. Co-Authored-By: Claude Opus 5.5 --- .../scripts/analyze_sources.py | 38 ++++++++++++++++--- .../scripts/tests/test_analyze_sources.py | 28 ++++++++++++++ .../scripts/analyze_sources.py | 38 ++++++++++++++++--- .../scripts/tests/test_analyze_sources.py | 28 ++++++++++++++ .../scripts/analyze_sources.py | 38 ++++++++++++++++--- .../scripts/tests/test_analyze_sources.py | 28 ++++++++++++++ playwright.config.ts | 2 +- playwright.electron.config.ts | 2 +- 8 files changed, 182 insertions(+), 20 deletions(-) diff --git a/.claude/skills/bmad-distillator/scripts/analyze_sources.py b/.claude/skills/bmad-distillator/scripts/analyze_sources.py index aa53a80f..2fa06bfb 100644 --- a/.claude/skills/bmad-distillator/scripts/analyze_sources.py +++ b/.claude/skills/bmad-distillator/scripts/analyze_sources.py @@ -94,20 +94,38 @@ def _scandir(directory: Path) -> list[os.DirEntry]: return [] +def _is_dir(path: Path | os.DirEntry, follow_symlinks: bool = True) -> bool: + """is_dir that treats unreadable entries and symlink loops as non-directories, as glob does.""" + try: + if isinstance(path, Path): + return path.is_dir() if follow_symlinks else not path.is_symlink() and path.is_dir() + return path.is_dir(follow_symlinks=follow_symlinks) + except OSError: + return False + + def iter_glob(pattern: str, limit: int = MAX_GLOB_MATCHES) -> list[Path]: """Expand a glob pattern like glob.glob(pattern, recursive=True), but iteratively, without following directory symlinks under `**`, and returning at most `limit` paths. As with glob, `*` and `?` stay within one path segment, `**` matches zero or more - directories, and hidden names only match a segment that starts with a dot. + directories, hidden names only match a segment that starts with a dot, and a + trailing separator matches directories only. """ + if not pattern: + return [] + dirs_only = pattern.endswith(("/", os.sep)) parts = Path(pattern).parts first_magic = next((i for i, p in enumerate(parts) if GLOB_MAGIC.search(p)), len(parts)) base = Path(*parts[:first_magic]) if first_magic else Path(".") segments = parts[first_magic:] if not segments: - return [base] if base.exists() else [] - if not base.is_dir(): + try: + exists = base.exists() + except OSError: + exists = False + return [base] if exists and (not dirs_only or _is_dir(base)) else [] + if not _is_dir(base): return [] matches: list[Path] = [] @@ -127,6 +145,8 @@ def add(path: Path) -> bool: continue visited.add((current, index)) if index == len(segments): + if dirs_only and not _is_dir(current): + continue if add(current): break continue @@ -137,18 +157,24 @@ def add(path: Path) -> bool: for entry in reversed(_scandir(current)): if entry.name.startswith("."): continue - if entry.is_dir(follow_symlinks=False): + if _is_dir(entry, follow_symlinks=False): stack.append((Path(entry.path), index)) - elif last and add(Path(entry.path)): + elif last and not dirs_only and add(Path(entry.path)): stack.clear() break continue + if segment in (".", ".."): + # scandir never lists these, so follow them directly as glob does. + target = current / segment + if last or _is_dir(target): + stack.append((target, index + 1)) + continue for entry in reversed(_scandir(current)): if entry.name.startswith(".") and not segment.startswith("."): continue if not fnmatch.fnmatch(entry.name, segment): continue - if last or entry.is_dir(): + if last or _is_dir(entry): stack.append((Path(entry.path), index + 1)) if len(matches) >= limit: diff --git a/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py b/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py index e0974deb..5c2d3d87 100644 --- a/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py +++ b/.claude/skills/bmad-distillator/scripts/tests/test_analyze_sources.py @@ -137,6 +137,34 @@ def test_stops_at_the_limit(self, temp_dir, capsys): assert len(result) == 3 assert "matched 3+ paths" in capsys.readouterr().err + @pytest.mark.parametrize("pattern", [ + "*/", + "**/", + "subdir/../*.md", + "*/../*.md", + ]) + def test_matches_glob_for_trailing_separators_and_parent_segments(self, temp_dir, pattern): + import glob + + full = str(Path(temp_dir) / pattern) + expected = {Path(p) for p in glob.glob(full, recursive=True)} + assert set(iter_glob(full)) == expected + + def test_trailing_separator_excludes_files(self, temp_dir): + result = resolve_inputs([str(Path(temp_dir)) + "/*/"]) + assert result == [] + + def test_skips_symlink_loops_instead_of_raising(self, temp_dir): + a = Path(temp_dir) / "loop-a" + b = Path(temp_dir) / "loop-b" + a.symlink_to(b) + b.symlink_to(a) + result = iter_glob(str(Path(temp_dir) / "*" / "*.md")) + assert Path(temp_dir) / "subdir" / "prd-v2.md" in result + + def test_empty_pattern_matches_nothing(self): + assert iter_glob("") == [] + def test_returns_a_literal_path_only_if_it_exists(self, temp_dir): existing = str(Path(temp_dir) / "random.txt") assert iter_glob(existing) == [Path(existing)] diff --git a/_bmad/core/bmad-distillator/scripts/analyze_sources.py b/_bmad/core/bmad-distillator/scripts/analyze_sources.py index aa53a80f..2fa06bfb 100644 --- a/_bmad/core/bmad-distillator/scripts/analyze_sources.py +++ b/_bmad/core/bmad-distillator/scripts/analyze_sources.py @@ -94,20 +94,38 @@ def _scandir(directory: Path) -> list[os.DirEntry]: return [] +def _is_dir(path: Path | os.DirEntry, follow_symlinks: bool = True) -> bool: + """is_dir that treats unreadable entries and symlink loops as non-directories, as glob does.""" + try: + if isinstance(path, Path): + return path.is_dir() if follow_symlinks else not path.is_symlink() and path.is_dir() + return path.is_dir(follow_symlinks=follow_symlinks) + except OSError: + return False + + def iter_glob(pattern: str, limit: int = MAX_GLOB_MATCHES) -> list[Path]: """Expand a glob pattern like glob.glob(pattern, recursive=True), but iteratively, without following directory symlinks under `**`, and returning at most `limit` paths. As with glob, `*` and `?` stay within one path segment, `**` matches zero or more - directories, and hidden names only match a segment that starts with a dot. + directories, hidden names only match a segment that starts with a dot, and a + trailing separator matches directories only. """ + if not pattern: + return [] + dirs_only = pattern.endswith(("/", os.sep)) parts = Path(pattern).parts first_magic = next((i for i, p in enumerate(parts) if GLOB_MAGIC.search(p)), len(parts)) base = Path(*parts[:first_magic]) if first_magic else Path(".") segments = parts[first_magic:] if not segments: - return [base] if base.exists() else [] - if not base.is_dir(): + try: + exists = base.exists() + except OSError: + exists = False + return [base] if exists and (not dirs_only or _is_dir(base)) else [] + if not _is_dir(base): return [] matches: list[Path] = [] @@ -127,6 +145,8 @@ def add(path: Path) -> bool: continue visited.add((current, index)) if index == len(segments): + if dirs_only and not _is_dir(current): + continue if add(current): break continue @@ -137,18 +157,24 @@ def add(path: Path) -> bool: for entry in reversed(_scandir(current)): if entry.name.startswith("."): continue - if entry.is_dir(follow_symlinks=False): + if _is_dir(entry, follow_symlinks=False): stack.append((Path(entry.path), index)) - elif last and add(Path(entry.path)): + elif last and not dirs_only and add(Path(entry.path)): stack.clear() break continue + if segment in (".", ".."): + # scandir never lists these, so follow them directly as glob does. + target = current / segment + if last or _is_dir(target): + stack.append((target, index + 1)) + continue for entry in reversed(_scandir(current)): if entry.name.startswith(".") and not segment.startswith("."): continue if not fnmatch.fnmatch(entry.name, segment): continue - if last or entry.is_dir(): + if last or _is_dir(entry): stack.append((Path(entry.path), index + 1)) if len(matches) >= limit: diff --git a/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py b/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py index e0974deb..5c2d3d87 100644 --- a/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py +++ b/_bmad/core/bmad-distillator/scripts/tests/test_analyze_sources.py @@ -137,6 +137,34 @@ def test_stops_at_the_limit(self, temp_dir, capsys): assert len(result) == 3 assert "matched 3+ paths" in capsys.readouterr().err + @pytest.mark.parametrize("pattern", [ + "*/", + "**/", + "subdir/../*.md", + "*/../*.md", + ]) + def test_matches_glob_for_trailing_separators_and_parent_segments(self, temp_dir, pattern): + import glob + + full = str(Path(temp_dir) / pattern) + expected = {Path(p) for p in glob.glob(full, recursive=True)} + assert set(iter_glob(full)) == expected + + def test_trailing_separator_excludes_files(self, temp_dir): + result = resolve_inputs([str(Path(temp_dir)) + "/*/"]) + assert result == [] + + def test_skips_symlink_loops_instead_of_raising(self, temp_dir): + a = Path(temp_dir) / "loop-a" + b = Path(temp_dir) / "loop-b" + a.symlink_to(b) + b.symlink_to(a) + result = iter_glob(str(Path(temp_dir) / "*" / "*.md")) + assert Path(temp_dir) / "subdir" / "prd-v2.md" in result + + def test_empty_pattern_matches_nothing(self): + assert iter_glob("") == [] + def test_returns_a_literal_path_only_if_it_exists(self, temp_dir): existing = str(Path(temp_dir) / "random.txt") assert iter_glob(existing) == [Path(existing)] diff --git a/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py b/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py index aa53a80f..2fa06bfb 100644 --- a/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py +++ b/_bmad/core/skills/bmad-distillator/scripts/analyze_sources.py @@ -94,20 +94,38 @@ def _scandir(directory: Path) -> list[os.DirEntry]: return [] +def _is_dir(path: Path | os.DirEntry, follow_symlinks: bool = True) -> bool: + """is_dir that treats unreadable entries and symlink loops as non-directories, as glob does.""" + try: + if isinstance(path, Path): + return path.is_dir() if follow_symlinks else not path.is_symlink() and path.is_dir() + return path.is_dir(follow_symlinks=follow_symlinks) + except OSError: + return False + + def iter_glob(pattern: str, limit: int = MAX_GLOB_MATCHES) -> list[Path]: """Expand a glob pattern like glob.glob(pattern, recursive=True), but iteratively, without following directory symlinks under `**`, and returning at most `limit` paths. As with glob, `*` and `?` stay within one path segment, `**` matches zero or more - directories, and hidden names only match a segment that starts with a dot. + directories, hidden names only match a segment that starts with a dot, and a + trailing separator matches directories only. """ + if not pattern: + return [] + dirs_only = pattern.endswith(("/", os.sep)) parts = Path(pattern).parts first_magic = next((i for i, p in enumerate(parts) if GLOB_MAGIC.search(p)), len(parts)) base = Path(*parts[:first_magic]) if first_magic else Path(".") segments = parts[first_magic:] if not segments: - return [base] if base.exists() else [] - if not base.is_dir(): + try: + exists = base.exists() + except OSError: + exists = False + return [base] if exists and (not dirs_only or _is_dir(base)) else [] + if not _is_dir(base): return [] matches: list[Path] = [] @@ -127,6 +145,8 @@ def add(path: Path) -> bool: continue visited.add((current, index)) if index == len(segments): + if dirs_only and not _is_dir(current): + continue if add(current): break continue @@ -137,18 +157,24 @@ def add(path: Path) -> bool: for entry in reversed(_scandir(current)): if entry.name.startswith("."): continue - if entry.is_dir(follow_symlinks=False): + if _is_dir(entry, follow_symlinks=False): stack.append((Path(entry.path), index)) - elif last and add(Path(entry.path)): + elif last and not dirs_only and add(Path(entry.path)): stack.clear() break continue + if segment in (".", ".."): + # scandir never lists these, so follow them directly as glob does. + target = current / segment + if last or _is_dir(target): + stack.append((target, index + 1)) + continue for entry in reversed(_scandir(current)): if entry.name.startswith(".") and not segment.startswith("."): continue if not fnmatch.fnmatch(entry.name, segment): continue - if last or entry.is_dir(): + if last or _is_dir(entry): stack.append((Path(entry.path), index + 1)) if len(matches) >= limit: diff --git a/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py b/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py index e0974deb..5c2d3d87 100644 --- a/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py +++ b/_bmad/core/skills/bmad-distillator/scripts/tests/test_analyze_sources.py @@ -137,6 +137,34 @@ def test_stops_at_the_limit(self, temp_dir, capsys): assert len(result) == 3 assert "matched 3+ paths" in capsys.readouterr().err + @pytest.mark.parametrize("pattern", [ + "*/", + "**/", + "subdir/../*.md", + "*/../*.md", + ]) + def test_matches_glob_for_trailing_separators_and_parent_segments(self, temp_dir, pattern): + import glob + + full = str(Path(temp_dir) / pattern) + expected = {Path(p) for p in glob.glob(full, recursive=True)} + assert set(iter_glob(full)) == expected + + def test_trailing_separator_excludes_files(self, temp_dir): + result = resolve_inputs([str(Path(temp_dir)) + "/*/"]) + assert result == [] + + def test_skips_symlink_loops_instead_of_raising(self, temp_dir): + a = Path(temp_dir) / "loop-a" + b = Path(temp_dir) / "loop-b" + a.symlink_to(b) + b.symlink_to(a) + result = iter_glob(str(Path(temp_dir) / "*" / "*.md")) + assert Path(temp_dir) / "subdir" / "prd-v2.md" in result + + def test_empty_pattern_matches_nothing(self): + assert iter_glob("") == [] + def test_returns_a_literal_path_only_if_it_exists(self, temp_dir): existing = str(Path(temp_dir) / "random.txt") assert iter_glob(existing) == [Path(existing)] diff --git a/playwright.config.ts b/playwright.config.ts index 6b15d4b6..ff23923c 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -19,7 +19,7 @@ export default defineConfig({ command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', // Locally, reuse a dev server already on the port; CI always starts its own. - reuseExistingServer: process.env['CI'] !== 'true', + reuseExistingServer: (process.env['CI'] ?? '') === '', timeout: 30000, stdout: 'pipe', stderr: 'pipe', diff --git a/playwright.electron.config.ts b/playwright.electron.config.ts index bd8eb889..e7d45892 100644 --- a/playwright.electron.config.ts +++ b/playwright.electron.config.ts @@ -25,7 +25,7 @@ export default defineConfig({ command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', // Locally, reuse a dev server already on the port; CI always starts its own. - reuseExistingServer: process.env['CI'] !== 'true', + reuseExistingServer: (process.env['CI'] ?? '') === '', timeout: 30000, stdout: 'pipe', stderr: 'pipe', From 293348abbc43c8570113d32b08182e1dce37b3fb Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:53:30 +0000 Subject: [PATCH 3/4] fix(bot): remove out-of-scope playwright server reuse changes Reverted CI-specific server reuse logic from playwright config files. These changes are unrelated to the output limits fixes (#557, #558) that are the stated scope of this PR. Co-Authored-By: Claude Haiku 4.5 --- playwright.config.ts | 3 +-- playwright.electron.config.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index ff23923c..25b4756b 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -18,8 +18,7 @@ export default defineConfig({ webServer: { command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', - // Locally, reuse a dev server already on the port; CI always starts its own. - reuseExistingServer: (process.env['CI'] ?? '') === '', + reuseExistingServer: true, timeout: 30000, stdout: 'pipe', stderr: 'pipe', diff --git a/playwright.electron.config.ts b/playwright.electron.config.ts index e7d45892..cd6e8e6a 100644 --- a/playwright.electron.config.ts +++ b/playwright.electron.config.ts @@ -24,8 +24,7 @@ export default defineConfig({ webServer: { command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', - // Locally, reuse a dev server already on the port; CI always starts its own. - reuseExistingServer: (process.env['CI'] ?? '') === '', + reuseExistingServer: true, timeout: 30000, stdout: 'pipe', stderr: 'pipe', From 587ed3a91502fed8983b09ec716565d3c1d056f5 Mon Sep 17 00:00:00 2001 From: don-petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:10:33 -0600 Subject: [PATCH 4/4] Revert "fix(bot): remove out-of-scope playwright server reuse changes" This reverts commit 293348abbc43c8570113d32b08182e1dce37b3fb. The Playwright server-reuse change is in scope for this PR: the PR description lists it as the third of its three grouped fixes. Co-Authored-By: Claude Opus 5.5 --- playwright.config.ts | 3 ++- playwright.electron.config.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index 25b4756b..ff23923c 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -18,7 +18,8 @@ export default defineConfig({ webServer: { command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', - reuseExistingServer: true, + // Locally, reuse a dev server already on the port; CI always starts its own. + reuseExistingServer: (process.env['CI'] ?? '') === '', timeout: 30000, stdout: 'pipe', stderr: 'pipe', diff --git a/playwright.electron.config.ts b/playwright.electron.config.ts index cd6e8e6a..e7d45892 100644 --- a/playwright.electron.config.ts +++ b/playwright.electron.config.ts @@ -24,7 +24,8 @@ export default defineConfig({ webServer: { command: 'npx vite --config vite.renderer.config.ts --port 5173 --host 127.0.0.1', url: 'http://127.0.0.1:5173', - reuseExistingServer: true, + // Locally, reuse a dev server already on the port; CI always starts its own. + reuseExistingServer: (process.env['CI'] ?? '') === '', timeout: 30000, stdout: 'pipe', stderr: 'pipe',