diff --git a/src/apm_cli/compilation/context_optimizer.py b/src/apm_cli/compilation/context_optimizer.py index b4e5f1c69..9cc8969ac 100644 --- a/src/apm_cli/compilation/context_optimizer.py +++ b/src/apm_cli/compilation/context_optimizer.py @@ -27,6 +27,26 @@ from ..primitives.models import Instruction from ..utils.exclude import should_exclude, validate_exclude_patterns from ..utils.paths import portable_relpath +from ..utils.patterns import parse_apply_to + + +def _has_top_level_comma(pattern: str) -> bool: + """Return True if ``pattern`` contains a comma outside any ``{...}`` group. + + Commas inside brace alternation (e.g. ``**/*.{css,scss}``) are part + of glob brace expansion and must not be treated as list separators. + """ + depth = 0 + for ch in pattern: + if ch == "{": + depth += 1 + elif ch == "}": + if depth > 0: + depth -= 1 + elif ch == "," and depth == 0: + return True + return False + # CRITICAL: Shadow Click commands to prevent namespace collision # When this module is imported during 'apm compile', Click's active context @@ -570,20 +590,21 @@ def _solve_placement_optimization( if not matching_directories: # Smart fallback: Try to place in semantically appropriate directory intended_dir = self._extract_intended_directory_from_pattern(pattern) + name = getattr(instruction, "name", None) or instruction.file_path.stem if intended_dir: # Place in the intended directory (e.g., docs/ for docs/**/*.md) placement = intended_dir reasoning = f"No matching files found, placed in intended directory '{portable_relpath(intended_dir, self.base_dir)}'" self._warnings.append( - f"Pattern '{pattern}' matches no files - placing in intended directory '{portable_relpath(intended_dir, self.base_dir)}'" + f"applyTo for '{name}' matched no files - placing in '{portable_relpath(intended_dir, self.base_dir)}'" ) else: # Fallback to root for global patterns placement = self.base_dir reasoning = "No matching files found, fallback to root placement" self._warnings.append( - f"Pattern '{pattern}' matches no files - placing at project root" + f"applyTo for '{name}' matched no files - placing at project root" ) # Calculate relevance score for the fallback placement @@ -659,11 +680,19 @@ def _extract_intended_directory_from_pattern(self, pattern: str) -> Path | None: """Extract the intended directory from a pattern like 'docs/**/*.md' -> 'docs'. Args: - pattern (str): File pattern to analyze. + pattern (str): File pattern (may be a comma-separated list). Returns: Optional[Path]: Intended directory path, or None if pattern is global. """ + # For comma-lists, only the first segment is consulted - the + # placement still flows into a single directory. + if _has_top_level_comma(pattern): + segments = parse_apply_to(pattern) + if not segments: + return None + pattern = segments[0] + if not pattern or pattern.startswith("**/"): return None # Global pattern @@ -711,11 +740,19 @@ def _file_matches_pattern(self, file_path: Path, pattern: str) -> bool: Args: file_path (Path): File path to check - pattern (str): Glob pattern to match against + pattern (str): Glob pattern or comma-separated list of globs. Returns: - bool: True if file matches pattern + bool: True if file matches pattern (or any segment of a list). """ + # applyTo accepts a comma-separated list of globs; treat any + # segment match as a hit so list patterns mirror per-glob semantics. + # Only split on top-level commas - commas inside brace alternation + # (e.g. ``**/*.{css,scss}``) must stay attached for brace expansion. + if _has_top_level_comma(pattern): + segments = parse_apply_to(pattern) + return any(self._file_matches_pattern(file_path, seg) for seg in segments) + # Expand any brace patterns expanded_patterns = self._expand_glob_pattern(pattern) diff --git a/src/apm_cli/integration/instruction_integrator.py b/src/apm_cli/integration/instruction_integrator.py index e4a5118c1..b5b4f18cf 100644 --- a/src/apm_cli/integration/instruction_integrator.py +++ b/src/apm_cli/integration/instruction_integrator.py @@ -16,6 +16,7 @@ from apm_cli.integration.base_integrator import BaseIntegrator, IntegrationResult from apm_cli.utils.path_security import ensure_path_within from apm_cli.utils.paths import portable_relpath +from apm_cli.utils.patterns import parse_apply_to if TYPE_CHECKING: from apm_cli.integration.targets import TargetProfile @@ -278,8 +279,12 @@ def _convert_to_cursor_rules(content: str) -> str: parts = ["---"] if description: parts.append(f"description: {description}") - if apply_to: - parts.append(f'globs: "{apply_to}"') + globs = parse_apply_to(apply_to) + if len(globs) == 1: + parts.append(f'globs: "{globs[0]}"') + elif globs: + parts.append("globs:") + parts.extend(f' - "{g}"' for g in globs) parts.append("---") return "\n".join(parts) + "\n\n" + body.lstrip("\n") @@ -366,12 +371,17 @@ def _convert_to_windsurf_rules(content: str) -> str: # Build Windsurf rules frontmatter parts = ["---"] - if apply_to: - # Sanitize: strip newlines to prevent frontmatter injection - # via crafted applyTo values (e.g. "**\ntrigger: always_on"). - safe_apply_to = apply_to.replace("\n", " ").replace("\r", " ").strip() + # Sanitize: strip newlines to prevent frontmatter injection + # via crafted applyTo values (e.g. "**\ntrigger: always_on"). + safe_apply_to = apply_to.replace("\n", " ").replace("\r", " ").strip() + globs = parse_apply_to(safe_apply_to) + if globs: parts.append("trigger: glob") - parts.append(f'globs: "{safe_apply_to}"') + if len(globs) == 1: + parts.append(f'globs: "{globs[0]}"') + else: + parts.append("globs:") + parts.extend(f' - "{g}"' for g in globs) else: parts.append("trigger: always_on") parts.append("---") @@ -420,10 +430,10 @@ def _convert_to_claude_rules(content: str) -> str: apply_to = line_stripped[len("applyTo:") :].strip().strip("'\"") # Build Claude rules frontmatter (only when path-scoped) - if apply_to: - parts = ["---"] - parts.append("paths:") - parts.append(f' - "{apply_to}"') + globs = parse_apply_to(apply_to) + if globs: + parts = ["---", "paths:"] + parts.extend(f' - "{g}"' for g in globs) parts.append("---") return "\n".join(parts) + "\n\n" + body.lstrip("\n") diff --git a/src/apm_cli/utils/patterns.py b/src/apm_cli/utils/patterns.py new file mode 100644 index 000000000..f2cce7a69 --- /dev/null +++ b/src/apm_cli/utils/patterns.py @@ -0,0 +1,44 @@ +"""Shared helpers for working with primitive ``applyTo`` patterns. + +The ``applyTo`` frontmatter on instruction primitives is documented as a +glob OR a comma-separated list of globs. This module owns the canonical +parse so converters and the placement optimizer behave consistently. +""" + +from __future__ import annotations + + +def parse_apply_to(value: str | None) -> list[str]: + """Split a primitive ``applyTo`` value into individual glob patterns. + + The input is either a single glob (``"**/*.py"``) or a + comma-separated list (``"**/src/**,**/api/**"``). Each segment is + stripped of surrounding whitespace; empty segments are discarded so + leading, trailing, doubled-up, and lone commas are tolerated. + + Commas inside brace alternation (``{a,b}``) are NOT separators -- only + top-level commas split the list. So ``"**/*.{css,scss},**/*.py"`` + yields ``["**/*.{css,scss}", "**/*.py"]``. + + Returns an empty list for ``None``, empty, or whitespace-only input. + """ + if not value: + return [] + segments: list[str] = [] + depth = 0 + current: list[str] = [] + for char in value: + if char == "{": + depth += 1 + current.append(char) + elif char == "}": + if depth > 0: + depth -= 1 + current.append(char) + elif char == "," and depth == 0: + segments.append("".join(current)) + current = [] + else: + current.append(char) + segments.append("".join(current)) + return [segment for segment in (s.strip() for s in segments) if segment] diff --git a/tests/integration/test_apply_to_comma_e2e.py b/tests/integration/test_apply_to_comma_e2e.py new file mode 100644 index 000000000..c714815b7 --- /dev/null +++ b/tests/integration/test_apply_to_comma_e2e.py @@ -0,0 +1,82 @@ +"""End-to-end test for comma-separated applyTo handling (issue #1366). + +Builds a single instruction primitive with a comma-separated ``applyTo`` +glob list and exercises each of the four target converters +(Copilot / Cursor / Windsurf / Claude), then asserts every segment ends +up in the rendered artifact in the target's native form. + +Copilot must preserve the value verbatim (consuming tool splits it); +the other three must emit a YAML list under their respective key. +""" + +import tempfile +from pathlib import Path + +import pytest + +from apm_cli.integration.instruction_integrator import InstructionIntegrator + +COMMA_APPLY_TO = "**/src/**,**/api/**,**/services/**" +SEGMENTS = ["**/src/**", "**/api/**", "**/services/**"] + + +@pytest.fixture +def source_instruction(): + """Write a primitive instruction file with comma-separated applyTo.""" + with tempfile.TemporaryDirectory() as td: + src = Path(td) / "multi.instructions.md" + src.write_text( + "---\n" + f"applyTo: '{COMMA_APPLY_TO}'\n" + "description: 'rules for src api services'\n" + "---\n" + "\n" + "# Multi-glob rules\n" + "\n" + "Body content.\n" + ) + yield src + + +def test_copilot_preserves_verbatim(source_instruction, tmp_path): + """Copilot target must keep the comma-list as-is.""" + dst = tmp_path / "copilot.instructions.md" + integrator = InstructionIntegrator() + integrator.copy_instruction(source_instruction, dst) + out = dst.read_text() + assert f"applyTo: '{COMMA_APPLY_TO}'" in out + + +def test_cursor_emits_yaml_list(source_instruction, tmp_path): + dst = tmp_path / "cursor.mdc" + integrator = InstructionIntegrator() + integrator.copy_instruction_cursor(source_instruction, dst) + out = dst.read_text() + assert "globs:" in out + for seg in SEGMENTS: + assert f' - "{seg}"' in out + # Make sure we did NOT emit the legacy literal comma string. + assert f'globs: "{COMMA_APPLY_TO}"' not in out + + +def test_windsurf_emits_yaml_list(source_instruction, tmp_path): + dst = tmp_path / "windsurf.md" + integrator = InstructionIntegrator() + integrator.copy_instruction_windsurf(source_instruction, dst) + out = dst.read_text() + assert "trigger: glob" in out + assert "globs:" in out + for seg in SEGMENTS: + assert f' - "{seg}"' in out + assert f'globs: "{COMMA_APPLY_TO}"' not in out + + +def test_claude_emits_yaml_list(source_instruction, tmp_path): + dst = tmp_path / "claude.md" + integrator = InstructionIntegrator() + integrator.copy_instruction_claude(source_instruction, dst) + out = dst.read_text() + assert "paths:" in out + for seg in SEGMENTS: + assert f' - "{seg}"' in out + assert f'paths: "{COMMA_APPLY_TO}"' not in out diff --git a/tests/unit/compilation/test_context_optimizer.py b/tests/unit/compilation/test_context_optimizer.py index 561afab0e..62818729d 100644 --- a/tests/unit/compilation/test_context_optimizer.py +++ b/tests/unit/compilation/test_context_optimizer.py @@ -898,5 +898,92 @@ def test_set_path_cached_across_calls(self): assert id(optimizer._glob_set_cache["**/*.ts"]) == cached_set_id +# ================================================================== +# Comma-separated applyTo handling in the optimizer (issue #1366) +# ================================================================== + + +class TestApplyToCommaInOptimizer: + """Verify the optimizer splits comma-separated applyTo globs.""" + + @pytest.fixture + def comma_project(self): + with tempfile.TemporaryDirectory() as td: + root = Path(td) + (root / "server").mkdir() + (root / "styles").mkdir() + (root / "tests").mkdir() + (root / "server" / "api.py").touch() + (root / "styles" / "main.css").touch() + (root / "tests" / "test_api.py").touch() + yield root + + def test_comma_list_partial_match_places_without_warning(self, comma_project): + """Comma-list where some segments match files: no 'no files' warning.""" + opt = ContextOptimizer(str(comma_project)) + instr = Instruction( + name="multi", + file_path=Path("multi.instructions.md"), + description="multi", + apply_to="**/*.py,**/*.never_matches", + content="rules", + source="local", + ) + placement = opt.optimize_instruction_placement([instr]) + assert placement, "instruction should be placed" + assert not any("matched no files" in w for w in opt._warnings) + assert not any("matches no files" in w for w in opt._warnings) + + def test_comma_list_zero_match_emits_single_warning(self, comma_project): + """Comma-list where NO segment matches: exactly one warning, names primitive.""" + opt = ContextOptimizer(str(comma_project)) + instr = Instruction( + name="orphan", + file_path=Path("orphan.instructions.md"), + description="orphan", + apply_to="**/*.nope,**/*.zilch", + content="rules", + source="local", + ) + opt.optimize_instruction_placement([instr]) + no_match_warnings = [ + w for w in opt._warnings if "matched no files" in w or "matches no files" in w + ] + assert len(no_match_warnings) == 1 + assert "orphan" in no_match_warnings[0] + # Warning must not echo the raw multi-pattern (noise reduction). + assert "**/*.nope,**/*.zilch" not in no_match_warnings[0] + + def test_comma_list_whitespace_trimmed(self, comma_project): + """Whitespace around comma segments is stripped before matching.""" + opt = ContextOptimizer(str(comma_project)) + instr = Instruction( + name="trimmed", + file_path=Path("trimmed.instructions.md"), + description="trimmed", + apply_to=" **/*.py , **/*.css ", + content="rules", + source="local", + ) + placement = opt.optimize_instruction_placement([instr]) + assert placement + assert not any("matched no files" in w for w in opt._warnings) + + def test_single_glob_regression(self, comma_project): + """Pre-existing single-glob behavior is unchanged.""" + opt = ContextOptimizer(str(comma_project)) + instr = Instruction( + name="py", + file_path=Path("py.instructions.md"), + description="py", + apply_to="**/*.py", + content="rules", + source="local", + ) + placement = opt.optimize_instruction_placement([instr]) + assert placement + assert not any("matched no files" in w for w in opt._warnings) + + if __name__ == "__main__": pytest.main([__file__]) diff --git a/tests/unit/integration/test_instruction_integrator.py b/tests/unit/integration/test_instruction_integrator.py index 923f31e67..ba0eca12c 100644 --- a/tests/unit/integration/test_instruction_integrator.py +++ b/tests/unit/integration/test_instruction_integrator.py @@ -1181,6 +1181,142 @@ def test_double_quoted_apply_to(self): assert 'globs: "src/**/*.ts"' in result +# ================================================================== +# Comma-separated applyTo splitting tests (issue #1366) +# Covers Claude, Cursor, Windsurf converters and Copilot verbatim path. +# ================================================================== + + +class TestApplyToCommaSplitting: + """Verify all three converters split comma-separated applyTo globs.""" + + # ---- Claude ---- + + def test_claude_comma_list_emits_yaml_list(self): + content = "---\napplyTo: '**/src/**,**/api/**,**/services/**'\n---\n\n# Rules" + result = InstructionIntegrator._convert_to_claude_rules(content) + assert "paths:" in result + assert ' - "**/src/**"' in result + assert ' - "**/api/**"' in result + assert ' - "**/services/**"' in result + + def test_claude_comma_whitespace_trimmed(self): + content = "---\napplyTo: '**/*.py, **/*.pyi , src/**/*.ts'\n---\n\n# R" + result = InstructionIntegrator._convert_to_claude_rules(content) + assert ' - "**/*.py"' in result + assert ' - "**/*.pyi"' in result + assert ' - "src/**/*.ts"' in result + assert ' - " **/*.pyi"' not in result + + def test_claude_trailing_comma_dropped(self): + content = "---\napplyTo: '**/*.py,'\n---\n\n# R" + result = InstructionIntegrator._convert_to_claude_rules(content) + assert "paths:" in result + assert ' - "**/*.py"' in result + assert result.count(' - "') == 1 + assert ' - ""' not in result + + def test_claude_leading_comma_dropped(self): + content = "---\napplyTo: ',**/*.py'\n---\n\n# R" + result = InstructionIntegrator._convert_to_claude_rules(content) + assert result.count(' - "') == 1 + assert ' - "**/*.py"' in result + + def test_claude_single_comma_treated_as_empty(self): + content = "---\napplyTo: ','\n---\n\n# R" + result = InstructionIntegrator._convert_to_claude_rules(content) + assert "paths:" not in result + assert "# R" in result + + def test_claude_whitespace_only_treated_as_empty(self): + content = "---\napplyTo: ' '\n---\n\n# R" + result = InstructionIntegrator._convert_to_claude_rules(content) + assert "paths:" not in result + assert "# R" in result + + # ---- Cursor ---- + + def test_cursor_comma_list_emits_yaml_list(self): + content = "---\napplyTo: '**/src/**,**/api/**,**/services/**'\n---\n\n# R" + result = InstructionIntegrator._convert_to_cursor_rules(content) + assert "globs:" in result + assert ' - "**/src/**"' in result + assert ' - "**/api/**"' in result + assert ' - "**/services/**"' in result + assert 'globs: "**/src/**,' not in result + + def test_cursor_single_glob_stays_scalar(self): + content = "---\napplyTo: '**/*.py'\n---\n\n# R" + result = InstructionIntegrator._convert_to_cursor_rules(content) + assert 'globs: "**/*.py"' in result + assert ' - "**/*.py"' not in result + + def test_cursor_comma_whitespace_trimmed(self): + content = "---\napplyTo: 'a, b , c'\n---\n\n# R" + result = InstructionIntegrator._convert_to_cursor_rules(content) + assert ' - "a"' in result + assert ' - "b"' in result + assert ' - "c"' in result + + def test_cursor_single_comma_treated_as_empty(self): + content = "---\napplyTo: ','\n---\n\n# R" + result = InstructionIntegrator._convert_to_cursor_rules(content) + assert "globs" not in result + + def test_cursor_trailing_comma_normalises_to_single(self): + content = "---\napplyTo: '**/*.py,'\n---\n\n# R" + result = InstructionIntegrator._convert_to_cursor_rules(content) + assert 'globs: "**/*.py"' in result + + # ---- Windsurf ---- + + def test_windsurf_comma_list_emits_yaml_list(self): + content = "---\napplyTo: '**/src/**,**/api/**,**/services/**'\n---\n\n# R" + result = InstructionIntegrator._convert_to_windsurf_rules(content) + assert "trigger: glob" in result + assert "globs:" in result + assert ' - "**/src/**"' in result + assert ' - "**/api/**"' in result + assert ' - "**/services/**"' in result + assert 'globs: "**/src/**,' not in result + + def test_windsurf_single_glob_stays_scalar(self): + content = "---\napplyTo: '**/*.py'\n---\n\n# R" + result = InstructionIntegrator._convert_to_windsurf_rules(content) + assert 'globs: "**/*.py"' in result + + def test_windsurf_comma_whitespace_trimmed(self): + content = '---\napplyTo: "a, b , c"\n---\n\n# R' + result = InstructionIntegrator._convert_to_windsurf_rules(content) + assert ' - "a"' in result + assert ' - "b"' in result + assert ' - "c"' in result + + def test_windsurf_single_comma_falls_back_to_always_on(self): + content = '---\napplyTo: ","\n---\n\n# R' + result = InstructionIntegrator._convert_to_windsurf_rules(content) + assert "trigger: always_on" in result + assert "globs" not in result + + # ---- Copilot verbatim preservation ---- + + def test_copilot_preserves_comma_list_verbatim(self): + """Copilot must NEVER split applyTo - consuming tool handles it.""" + import tempfile + + with tempfile.TemporaryDirectory() as td: + src = Path(td) / "src.instructions.md" + dst = Path(td) / "dst.instructions.md" + src.write_text( + "---\napplyTo: '**/src/**,**/api/**,**/services/**'\n---\n\n# Multi rules\n" + ) + integrator = InstructionIntegrator() + integrator.copy_instruction(src, dst) + written = dst.read_text() + assert "applyTo: '**/src/**,**/api/**,**/services/**'" in written + assert "paths:" not in written + + class TestWindsurfRulesIntegration: """Test end-to-end Windsurf rules deployment.""" diff --git a/tests/unit/utils/test_patterns.py b/tests/unit/utils/test_patterns.py new file mode 100644 index 000000000..f7148915f --- /dev/null +++ b/tests/unit/utils/test_patterns.py @@ -0,0 +1,57 @@ +"""Tests for the applyTo pattern parser.""" + +from apm_cli.utils.patterns import parse_apply_to + + +class TestParseApplyTo: + """Unit tests for parse_apply_to().""" + + def test_empty_string_returns_empty_list(self): + assert parse_apply_to("") == [] + + def test_whitespace_only_returns_empty_list(self): + assert parse_apply_to(" ") == [] + + def test_single_glob_returns_one_element(self): + assert parse_apply_to("**/*.py") == ["**/*.py"] + + def test_comma_list_split(self): + assert parse_apply_to("a,b,c") == ["a", "b", "c"] + + def test_whitespace_trimmed(self): + assert parse_apply_to("a, b , c") == ["a", "b", "c"] + + def test_trailing_comma_dropped(self): + assert parse_apply_to("a,b,") == ["a", "b"] + + def test_leading_comma_dropped(self): + assert parse_apply_to(",a,b") == ["a", "b"] + + def test_single_comma_returns_empty(self): + assert parse_apply_to(",") == [] + + def test_internal_empty_segments_dropped(self): + assert parse_apply_to("a, ,b") == ["a", "b"] + + def test_realistic_multi_glob(self): + assert parse_apply_to("**/src/**,**/api/**,**/services/**") == [ + "**/src/**", + "**/api/**", + "**/services/**", + ] + + def test_brace_alternation_not_split(self): + # Commas inside {...} are glob brace expansion, not list separators. + assert parse_apply_to("**/*.{css,scss}") == ["**/*.{css,scss}"] + + def test_brace_alternation_mixed_with_top_level_comma(self): + assert parse_apply_to("**/*.{css,scss},**/*.py") == [ + "**/*.{css,scss}", + "**/*.py", + ] + + def test_nested_braces(self): + assert parse_apply_to("**/{a,{b,c}},**/*.py") == [ + "**/{a,{b,c}}", + "**/*.py", + ]