Skip to content
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Comma-separated `applyTo` globs now correctly emit a YAML list under `globs:` / `paths:` in Claude, Cursor, and Windsurf instruction outputs (Copilot output unchanged -- it already preserves `applyTo` verbatim and handles comma-lists natively). The shared `parse_apply_to()` helper is brace-aware, so `**/*.{css,scss},**/*.py` is split on the top-level comma only. Closes a documented promise that was silently broken on the three non-Copilot harnesses. (#1387, closes #1366)
- `apm update` against private Azure DevOps deps no longer fails on Windows with a misleading "az present but not logged in" diagnostic when the user IS signed in via `az login`. Root cause: Python's `subprocess.run(["az", ...])` -> `CreateProcessW` does not honor `PATHEXT` for non-`.exe` executables, so the Windows `az.cmd` wrapper could not be invoked even though `shutil.which("az")` resolved it. `AzureCliBearerProvider` now resolves the `az` binary via `shutil.which` once at construction and passes the absolute path to every subprocess call. As a defense-in-depth measure, the ADO `--update` preflight probe no longer strips `GIT_CONFIG_GLOBAL` / `GIT_CONFIG_NOSYSTEM` / `GIT_ASKPASS`, so Git Credential Manager can answer for Entra-cached ADO credentials whenever bearer acquisition is unavailable for any reason (sandbox, proxy, future PATH quirks). The actual clone path keeps its full gitconfig isolation. (#1430)
- Root `.apm` hooks no longer duplicate after renaming the project directory or using git worktrees; Claude, Codex, Cursor, Gemini, and Windsurf hook configs stay idempotent across checkouts. The hook source-id is now derived from `apm.yml`'s `name` field instead of `install_path.name`, and `apm install` silently heals stale same-content entries from prior checkout basenames. Copilot is unaffected (its hooks live in per-file namespaces under `.github/hooks/`, not a shared merged config). (#1392, closes #1329)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,22 @@ unconditional and gets folded into compiled context files
(`AGENTS.md`, `GEMINI.md`) instead of a per-file rule directory. With
it, each harness wraps the body in its own scoping syntax.

`applyTo` accepts either a single glob or a comma-separated list of
globs. Commas inside brace alternation (`**/*.{css,scss}`) are part of
the glob and are NOT treated as list separators -- only top-level
commas split the list.

```markdown
---
description: Frontend style rules
applyTo: "**/*.{css,scss},**/*.tsx"
---
```

On Copilot the comma-list is preserved verbatim (Copilot splits it
natively). On Claude, Cursor, and Windsurf the list is expanded to a
YAML array under `paths:` / `globs:`.

### Body conventions

- Lead with bullets, not prose. Instructions are read by an agent
Expand All @@ -72,10 +88,10 @@ it, each harness wraps the body in its own scoping syntax.

| Target | Output path | Transform |
|---|---|---|
| copilot | `.github/instructions/<name>.instructions.md` | verbatim; `applyTo` preserved |
| claude | `.claude/rules/<name>.md` | `applyTo` -> `paths:` list |
| cursor | `.cursor/rules/<name>.mdc` | `applyTo` -> `globs:`; description auto-derived if missing |
| windsurf | `.windsurf/rules/<name>.md` | `applyTo` -> `trigger: glob` + `globs:`; missing `applyTo` -> `trigger: always_on` |
| copilot | `.github/instructions/<name>.instructions.md` | verbatim; `applyTo` preserved (comma-lists split natively by Copilot) |
| claude | `.claude/rules/<name>.md` | `applyTo` -> `paths:` list (comma-lists expanded to YAML array) |
| cursor | `.cursor/rules/<name>.mdc` | `applyTo` -> `globs:` (scalar for single glob, YAML array for comma-lists); description auto-derived if missing |
| windsurf | `.windsurf/rules/<name>.md` | `applyTo` -> `trigger: glob` + `globs:` (scalar or YAML array); missing `applyTo` -> `trigger: always_on` |
| codex | folded into `AGENTS.md` | compile-only, no per-file deploy |
| gemini | folded into `GEMINI.md` | compile-only, no per-file deploy |
| opencode | folded into `AGENTS.md` | compile-only, no per-file deploy |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ tags: [security, validation]
---
```

`applyTo` accepts a single glob (`"**/*.py"`) or a comma-separated list
(`"**/src/**,**/api/**"`). Commas inside brace alternation
(`**/*.{css,scss}`) are part of the glob and are NOT separators -- only
top-level commas split the list. On Copilot the value is preserved
verbatim; on Claude/Cursor/Windsurf comma-lists are expanded to a YAML
array under `paths:` / `globs:`.

### 2. Chatmode (`*.chatmode.md`)

Chat persona configuration.
Expand Down
25 changes: 3 additions & 22 deletions src/apm_cli/compilation/context_optimizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,7 @@
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

from ..utils.patterns import has_top_level_comma, parse_apply_to

# CRITICAL: Shadow Click commands to prevent namespace collision
# When this module is imported during 'apm compile', Click's active context
Expand Down Expand Up @@ -687,7 +668,7 @@ def _extract_intended_directory_from_pattern(self, pattern: str) -> Path | None:
"""
# For comma-lists, only the first segment is consulted - the
# placement still flows into a single directory.
if _has_top_level_comma(pattern):
if has_top_level_comma(pattern):
segments = parse_apply_to(pattern)
if not segments:
return None
Expand Down Expand Up @@ -749,7 +730,7 @@ def _file_matches_pattern(self, file_path: Path, pattern: str) -> bool:
# 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):
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 Down
12 changes: 6 additions & 6 deletions src/apm_cli/integration/instruction_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +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
from apm_cli.utils.patterns import parse_apply_to, yaml_double_quote

if TYPE_CHECKING:
from apm_cli.integration.targets import TargetProfile
Expand Down Expand Up @@ -282,10 +282,10 @@ def _convert_to_cursor_rules(content: str) -> str:
parts.append(f"description: {description}")
globs = parse_apply_to(apply_to)
if len(globs) == 1:
parts.append(f'globs: "{globs[0]}"')
parts.append(f"globs: {yaml_double_quote(globs[0])}")
elif globs:
parts.append("globs:")
parts.extend(f' - "{g}"' for g in globs)
parts.extend(f" - {yaml_double_quote(g)}" for g in globs)
parts.append("---")

return "\n".join(parts) + "\n\n" + body.lstrip("\n")
Expand Down Expand Up @@ -379,10 +379,10 @@ def _convert_to_windsurf_rules(content: str) -> str:
if globs:
parts.append("trigger: glob")
if len(globs) == 1:
parts.append(f'globs: "{globs[0]}"')
parts.append(f"globs: {yaml_double_quote(globs[0])}")
else:
parts.append("globs:")
parts.extend(f' - "{g}"' for g in globs)
parts.extend(f" - {yaml_double_quote(g)}" for g in globs)
else:
parts.append("trigger: always_on")
parts.append("---")
Expand Down Expand Up @@ -434,7 +434,7 @@ def _convert_to_claude_rules(content: str) -> str:
globs = parse_apply_to(apply_to)
if globs:
parts = ["---", "paths:"]
parts.extend(f' - "{g}"' for g in globs)
parts.extend(f" - {yaml_double_quote(g)}" for g in globs)
parts.append("---")
return "\n".join(parts) + "\n\n" + body.lstrip("\n")

Expand Down
47 changes: 47 additions & 0 deletions src/apm_cli/utils/patterns.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,53 @@
from __future__ import annotations


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.
Single source of truth for the comma-vs-brace discrimination; the
placement optimizer and the integrators both consume this so the
semantics of ``parse_apply_to`` and its callers stay in lock-step.
Comment on lines +16 to +18
"""
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


def yaml_double_quote(value: str) -> str:
"""Escape ``value`` for embedding inside a YAML double-quoted scalar.

Defence-in-depth for the instruction integrators that emit YAML
frontmatter via f-strings (``f' - "{g}"'``). A glob containing a
literal backslash, double-quote, or control character would break
the surrounding YAML if inlined verbatim; this helper escapes the
minimal set needed for the YAML 1.2 double-quoted form. Returns the
value already wrapped in the surrounding double quotes.
Comment on lines +32 to +40

Note: ``parse_apply_to`` already strips leading/trailing whitespace
per segment, and the Windsurf integrator strips newlines from the
raw frontmatter value before splitting, so the practical risk today
is near-zero -- this exists so emitted YAML stays well-formed even
on adversarial or copy-paste-mangled inputs.
"""
escaped = (
value.replace("\\", "\\\\")
.replace('"', '\\"')
.replace("\n", "\\n")
.replace("\r", "\\r")
.replace("\t", "\\t")
)
return f'"{escaped}"'


def parse_apply_to(value: str | None) -> list[str]:
"""Split a primitive ``applyTo`` value into individual glob patterns.

Expand Down
54 changes: 53 additions & 1 deletion tests/unit/utils/test_patterns.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Tests for the applyTo pattern parser."""

from apm_cli.utils.patterns import parse_apply_to
from apm_cli.utils.patterns import has_top_level_comma, parse_apply_to, yaml_double_quote


class TestParseApplyTo:
Expand Down Expand Up @@ -55,3 +55,55 @@ def test_nested_braces(self):
"**/{a,{b,c}}",
"**/*.py",
]


class TestHasTopLevelComma:
"""Unit tests for has_top_level_comma()."""

def test_no_comma(self):
assert has_top_level_comma("**/*.py") is False

def test_top_level_comma(self):
assert has_top_level_comma("a,b") is True

def test_brace_comma_only(self):
# Commas inside {...} are brace expansion, not separators.
assert has_top_level_comma("**/*.{css,scss}") is False

def test_brace_comma_and_top_level(self):
assert has_top_level_comma("**/*.{css,scss},**/*.py") is True

def test_nested_braces(self):
assert has_top_level_comma("**/{a,{b,c}}") is False

def test_empty(self):
assert has_top_level_comma("") is False


class TestYamlDoubleQuote:
"""Unit tests for yaml_double_quote() defence-in-depth escaping."""

def test_plain_glob(self):
assert yaml_double_quote("**/*.py") == '"**/*.py"'

def test_escapes_double_quote(self):
assert yaml_double_quote('a"b') == '"a\\"b"'

def test_escapes_backslash(self):
assert yaml_double_quote("a\\b") == '"a\\\\b"'

def test_escapes_newline(self):
assert yaml_double_quote("a\nb") == '"a\\nb"'

def test_escapes_carriage_return(self):
assert yaml_double_quote("a\rb") == '"a\\rb"'

def test_escapes_tab(self):
assert yaml_double_quote("a\tb") == '"a\\tb"'

def test_yaml_safe_load_roundtrip(self):
import yaml

for value in ['a"b', "a\\b", "a\nb", "**/src/**", "**/*.{css,scss}"]:
yaml_doc = f"k: {yaml_double_quote(value)}\n"
assert yaml.safe_load(yaml_doc) == {"k": value}
Loading