Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 42 additions & 5 deletions src/apm_cli/compilation/context_optimizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Comment on lines +748 to +754

# Expand any brace patterns
expanded_patterns = self._expand_glob_pattern(pattern)

Expand Down
32 changes: 21 additions & 11 deletions src/apm_cli/integration/instruction_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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("---")
Expand Down Expand Up @@ -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")

Expand Down
23 changes: 23 additions & 0 deletions src/apm_cli/utils/patterns.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
"""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.

Returns an empty list for ``None``, empty, or whitespace-only input.
"""
if not value:
return []
return [segment for segment in (part.strip() for part in value.split(",")) if segment]
Comment on lines +11 to +23
82 changes: 82 additions & 0 deletions tests/integration/test_apply_to_comma_e2e.py
Original file line number Diff line number Diff line change
@@ -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
87 changes: 87 additions & 0 deletions tests/unit/compilation/test_context_optimizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__])
Loading
Loading