Skip to content

fix(cursor): comma-joined bare globs/description in .mdc frontmatter - #3011

Open
Yannick Guyomar (YGuyomar) wants to merge 1 commit into
microsoft:mainfrom
YGuyomar:fix/cursor-globs-comma-join-3002
Open

Yannick Guyomar (YGuyomar) wants to merge 1 commit into
microsoft:mainfrom
YGuyomar:fix/cursor-globs-comma-join-3002

Conversation

@YGuyomar

@YGuyomar Yannick Guyomar (YGuyomar) commented Sep 17, 2026

Copy link
Copy Markdown

fix(cursor): comma-joined bare globs/description in .mdc frontmatter

TL;DR

The Cursor target compiler emitted globs as a quoted YAML list (globs:\n - "a"\n - "b") whenever an instruction's applyTo had comma-separated globs, and always double-quoted description/globs with \uXXXX-escaped non-ASCII text. Neither shape appears anywhere in Cursor's own .mdc docs. This PR makes Cursor emit the one documented shape: a single, comma-joined, always-bare scalar for globs, and a bare-when-possible, never-\u-escaped scalar for description.

Problem (WHY)

  • Cursor's docs show globs as a single string, multiple patterns comma-joined, never a YAML list, and never quoted -- not even for patterns starting with ** (globs: src/components/**/*.tsx, docs/**/*.md, docs/**/*.mdx). APM instead emitted a multi-item block sequence for 2+ globs, with every value double-quoted.
  • ensure_ascii=True on the old double-quoting also meant any non-ASCII description (em dash, arrows, accented text) round-tripped through \uXXXX escapes instead of literal UTF-8, hurting diff readability of generated .mdc files.
  • Root cause per the issue: the [BUG] applyTo comma-separated globs are not split ? Claude target emits broken paths: and compiler warns "matches no files" #1366 fix taught Claude's paths: to expand comma-lists into a YAML block sequence, and the same list-shaping logic was reused for Cursor's globs: "for consistency," without checking that Cursor's native format has no list syntax -- and no quoting -- for globs at all.

Fixes #3002.

Approach (WHAT)

Deviation reported Fix
YAML list for 2+ globs Always comma-join into one scalar (', '.join(globs)); list branch removed entirely
Quoted globs, including a leading ** New yaml_globs_scalar(): always bare. A leading ** is technically a YAML alias indicator under a strict parser, but Cursor's docs never quote it and its frontmatter reader tolerates it; glob syntax has none of the ": "/reserved-word ambiguity free text can have. Only a literal embedded newline still forces quoting (defence against frontmatter injection)
Quoted description, \uXXXX-escaped non-ASCII New yaml_plain_scalar(): bare if it round-trips through YAML's own resolver, else single-quoted, else (embedded newline only) double-quoted with ensure_ascii=False

Note

An earlier revision of this PR still single-quoted globs values starting with ** (treating a strict-YAML round-trip check as the safety bar). @YGuyomar caught that this contradicts Cursor's own docs, which never show a quoted globs value under any circumstance -- fixed by giving globs its own always-bare rule instead of reusing the description helper.

Scoped to the Cursor converter only. Claude/Windsurf/Kiro/Antigravity keep their existing (correct-for-them) always-quoted, list-capable format via the untouched yaml_double_quote() -- this issue is Cursor-specific, and touching the shared multi-target helper wasn't warranted.

Implementation (HOW)

  • src/apm_cli/utils/patterns.py: added yaml_plain_scalar(value) for description (bare-if-safe -> single-quoted -> double-quoted UTF-8 fallback, verified via yaml.safe_load) and yaml_globs_scalar(value) for globs (always bare unless the value contains a literal \n/\r, in which case JSON-escape it).
  • src/apm_cli/integration/instruction_integrator.py: _convert_to_cursor_rules() emits globs: {yaml_globs_scalar(', '.join(globs))} for any non-empty glob list (previously branched scalar-vs-list-of-quoted-scalars), and description goes through yaml_plain_scalar().
  • Tests: updated TestConvertToCursorRules, the Cursor rows of TestApplyToCommaSplitting (issue [BUG] applyTo comma-separated globs are not split ? Claude target emits broken paths: and compiler warns "matches no files" #1366's coverage), test_apply_to_comma_e2e.py, and test_integration_runtime_coverage.py to assert the bare, comma-joined shape -- including for globs starting with **. Added TestYamlPlainScalar (description helper) and TestYamlGlobsScalar (globs helper, including the "stays bare with a leading **" and "newline still quotes" cases).
  • Docs: instructions-and-agents.md's target transform table updated to describe the comma-joined, always-bare shape.

Trade-offs

  • globs and description now follow deliberately different quoting rules (always-bare vs. bare-when-safe). This is intentional, not an inconsistency: glob syntax has no legitimate need for the YAML ambiguities (: , reserved words, alias-like leading *) that free-form description text can hit, and Cursor's docs draw exactly that line -- every globs example is bare, description examples are plain English that happens not to need quoting.
  • Kept yaml_double_quote() untouched and added two narrowly-scoped helpers rather than generalizing one function, so Claude/Windsurf/Kiro/Antigravity behavior (out of scope here) can't regress by accident.
  • Did not touch the OpenAPM spec: the Mode B critical-path diff under src/apm_cli/integration/ is well under the 20-line threshold, and no existing req-tg-* statement pins Cursor's globs shape (only Antigravity's is normatively pinned today).

Validation

uv run python -m pytest tests/unit/utils/test_patterns.py tests/unit/integration/test_instruction_integrator.py tests/integration/test_apply_to_comma_e2e.py tests/integration/test_local_install.py tests/integration/test_integration_runtime_coverage.py -q
5 failed, 386 passed, 4 skipped

uv run ruff check <changed files>
All checks passed!
uv run ruff format <changed files>
6 files left unchanged
The 5 failures are pre-existing, reproduced identically on the pre-fix commit

Reran the exact same 5 tests with this PR's changes git stashed (i.e. against the prior commit) -- identical failures, none touching globs/cursor/patterns.py/instruction_integrator.py:

  • test_install_rejects_yaml_escaped_hidden_unicode_before_writes[surrogate-pair-tag] -- pre-existing Windows 'utf-8' codec can't encode ... surrogates not allowed in an unrelated unicode-security test.
  • test_deps_shows_local_packages -- unrelated subprocess.run result handling.
  • test_token_file_mode_ok_missing_file_returns_false -- unrelated Windows file-permission-mode test.
  • test_returns_output_and_return_code, test_failed_command_returns_nonzero_rc -- unrelated: call echo/false, which aren't standalone executables on Windows.

Before / after

# Before (issue #3002)
---
description: "Python service patterns"
globs:
  - "services/**/*.py"
  - "plugins/services/**/*.py"
---

# After
---
description: Python service patterns
globs: services/**/*.py, plugins/services/**/*.py
---
# Before -- a glob starting with "**" was double-quoted
globs:
  - "**/*.ts"

# After -- bare, matching every globs example in Cursor's docs
globs: **/*.ts

How to test

  1. apm install --target cursor a package whose instruction has a comma-separated applyTo (e.g. services/**/*.py,plugins/services/**/*.py).
  2. Open the generated .cursor/rules/<name>.mdc and confirm globs: is one unquoted, comma-joined line -- not a block sequence, not quoted.
  3. Repeat with an applyTo glob starting with ** (e.g. **/*.ts) and confirm it stays bare too.
  4. Use a description containing non-ASCII text (em dash, accented characters) and confirm it appears as literal UTF-8, not \uXXXX escapes.

@YGuyomar

Yannick Guyomar (YGuyomar) commented Sep 17, 2026 via email

Copy link
Copy Markdown
Author

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Three moderate implementation findings and one documentation inconsistency remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates Cursor .mdc frontmatter to emit comma-joined YAML scalar globs and readable UTF-8 descriptions.

Changes:

  • Added yaml_plain_scalar() and updated Cursor conversion.
  • Updated unit and integration coverage.
  • Updated documentation and changelog.

Review findings:

  • docs/src/content/docs/producer/author-primitives/instructions-and-agents.mdnit (3 votes): update the maintained usage skill’s outdated Cursor guidance.
  • src/apm_cli/integration/instruction_integrator.pymoderate (2 votes): preserve escaped literal commas when joining globs.
  • src/apm_cli/utils/patterns.pymoderate (3 votes): use the bounded YAML loader.
  • src/apm_cli/utils/patterns.pymoderate (1 vote): escape YAML-invalid or non-preserving control characters.
File summaries
File Description
tests/unit/utils/test_patterns.py Tests YAML scalar formatting and round trips.
tests/unit/integration/test_instruction_integrator.py Updates Cursor conversion expectations.
tests/integration/test_local_install.py Updates local-install expectations.
tests/integration/test_integration_runtime_coverage.py Updates runtime coverage expectations.
tests/integration/test_apply_to_comma_e2e.py Verifies comma-joined Cursor output.
src/apm_cli/utils/patterns.py Adds YAML scalar rendering.
src/apm_cli/integration/instruction_integrator.py Emits Cursor-compatible frontmatter.
docs/src/content/docs/producer/author-primitives/instructions-and-agents.md Documents the new Cursor format.
CHANGELOG.md Records the fix.
Review details

Suppressed comments (1)

src/apm_cli/utils/patterns.py:122

  • The ensure_ascii=False fallback is not sufficient for every YAML value: it leaves YAML-invalid controls such as U+007F unescaped, and leaves U+2028/U+2029 as YAML line breaks. A valid description containing one of these characters can therefore produce frontmatter that fails to parse or changes the text on a round trip. Escape YAML-invalid/non-preserving code points while keeping ordinary non-ASCII literal, and add regression coverage.
    return json.dumps(value, ensure_ascii=False)
  • Files reviewed: 9/9 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

parts.append("globs:")
parts.extend(f" - {yaml_double_quote(g)}" for g in globs)
if globs:
parts.append(f"globs: {yaml_plain_scalar(', '.join(globs))}")

@YGuyomar Yannick Guyomar (YGuyomar) Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7bb0169. _convert_to_cursor_rules now re-escapes each glob with the newly-public escape_apply_to_segment() (patterns.py) before joining with , , so applyTo: 'src/foo\,bar/*.py' renders as globs: src/foo\,bar/*.py instead of silently collapsing into two patterns.

Worth being explicit about the residual limitation: this can't make the pattern actually work in Cursor, since Cursor's own comma-splitting isn't escape-aware. What it does do is keep APM's own output consistent with parse_apply_to's documented convention instead of silently mis-splitting it -- and package-authoring.md plus the commit message now call out that a literal comma in a glob still isn't representable in a Cursor rule.

Added TestEscapeApplyToSegment and test_escaped_literal_comma_in_glob_survives_the_comma_join for regression coverage.

Comment thread src/apm_cli/utils/patterns.py Outdated
def _yaml_scalar_round_trips(candidate: str, expected: str) -> bool:
"""Return True if parsing ``candidate`` as a bare YAML scalar yields ``expected``."""
try:
return yaml.safe_load(candidate) == expected

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7bb0169. _yaml_scalar_round_trips now calls load_yaml_str() (the project's _BoundedSafeLoader-backed helper from yaml_io.py) instead of stock yaml.safe_load, so a hostile description can no longer bypass the repo's merge/alias expansion guard on this second, in-memory parse.

Added test_round_trip_check_stays_bounded_for_flow_style_alias_bomb, which feeds a single-line flow-style merge-key bomb (same shape as test_yaml_io.py::test_merge_key_bomb_fails_closed, just written without newlines so it isn't trivially short-circuited by the newline-forces-double-quote branch) through yaml_plain_scalar() and asserts it fails closed instead of attempting exponential alias expansion.

| claude | `.claude/rules/<name>.md` | `applyTo` -> `paths:` list (comma-lists expanded to YAML array) |
| grok-build | `.grok/rules/<name>.md` and folded into `AGENTS.md` | native rule plus compiled root context |
| cursor | `.cursor/rules/<name>.mdc` | `applyTo` -> `globs:` (scalar for single glob, YAML array for comma-lists); description auto-derived if missing |
| cursor | `.cursor/rules/<name>.mdc` | `applyTo` -> `globs:` (one comma-joined scalar, plain/unquoted when safe, matching Cursor's native format); description auto-derived if missing |

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7bb0169. Updated packages/apm-guide/.apm/skills/apm-usage/package-authoring.md's applyTo/globs section to describe Cursor's actual comma-joined bare-scalar globs: format (not a YAML array like Claude/Windsurf/Kiro/Antigravity), and added a note that a literal comma inside one glob still isn't representable in a Cursor rule even though APM's own tooling preserves the \, escape when re-joining patterns for that target.

@YGuyomar
Yannick Guyomar (YGuyomar) force-pushed the fix/cursor-globs-comma-join-3002 branch from 353313d to 4cea9e2 Compare September 17, 2026 12:19
@YGuyomar Yannick Guyomar (YGuyomar) changed the title fix(cursor): comma-joined plain-scalar globs/description in .mdc frontmatter fix(cursor): comma-joined bare globs/description in .mdc frontmatter Sep 17, 2026
@YGuyomar
Yannick Guyomar (YGuyomar) force-pushed the fix/cursor-globs-comma-join-3002 branch from 4cea9e2 to 7bb0169 Compare September 17, 2026 15:30
@YGuyomar

Copy link
Copy Markdown
Author

Also addressing the suppressed finding on src/apm_cli/utils/patterns.py:122 (no inline thread exists for it, so replying here): "The ensure_ascii=False fallback ... leaves YAML-invalid controls such as U+007F unescaped, and leaves U+2028/U+2029 as YAML line breaks ... Escape YAML-invalid/non-preserving code points while keeping ordinary non-ASCII literal, and add regression coverage."

Fixed in 7bb0169. The double-quoted fallback (_yaml_double_quote_utf8_safe()) now explicitly \uXXXX-escapes DEL (U+007F), NEL (U+0085), LS (U+2028), and PS (U+2029) on top of json.dumps(..., ensure_ascii=False), since JSON's own required-escape range stops at U+001F and doesn't cover any of these.

While adding regression coverage I found the round-trip check itself (not just the fallback) had a related blind spot: PyYAML folds any line-break-like character, not just \n/\r, when scanning a multi-line plain/single-quoted scalar -- so a value containing e.g. U+2029 immediately followed by \n could spuriously round-trip as "equal to itself" by coincidence, skipping the fallback entirely. yaml_plain_scalar()/yaml_globs_scalar() now check for any of these characters up front and route straight to the escaped fallback rather than relying purely on the round-trip check.

That same pass also surfaced a real regression against test_local_install.py::test_install_rejects_yaml_escaped_hidden_unicode_before_writes: a lone (unpaired) UTF-16 surrogate -- this project's existing hidden-unicode security-gate attack shape, e.g. a description escaped as \uDB40\uDC01 -- isn't just YAML-unsafe, it can't be UTF-8 encoded at all, so leaving it literal crashed the eventual file write outright even under --force. Folded the same fix in: lone surrogates now also force the escaped double-quoted fallback.

New coverage: test_del_control_char_escapes_to_uXXXX, test_line_separator_forces_double_quote_and_escapes, test_paragraph_separator_adjacent_to_newline_round_trips, test_next_line_control_char_escapes_to_uXXXX, test_lone_surrogate_escapes_instead_of_crashing_utf8_encode (both yaml_plain_scalar and yaml_globs_scalar variants), plus an integration-level test_lone_surrogate_description_does_not_crash_utf8_encode.

@sergio-sisternes-epam

Sergio Sisternes (sergio-sisternes-epam) commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Thank you for contributing this pull request, Yannick Guyomar (@YGuyomar), and for
filing #3002. APM starts with an issue, not an implementation
(https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md).

#3002 is not yet status/accepted and has no human scope record.
Please wait for a maintainer to review and accept that issue first.
This PR is labelled status/deferred until that happens.


Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors.

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) added triage/recommended Automated advice completed; not human scope approval. status/deferred Not invited for implementation now; no release commitment. type/bug Something does not work as documented. area/multi-target Multi-target deploy spec, target directory creation, agent surface routing. theme/portability One manifest, every target. Multi-target deploy, marketplace, packaging, install. status/accepted Human scope approval; verify the issue's approval record and review contact before work. and removed status/deferred Not invited for implementation now; no release commitment. labels Sep 18, 2026
@YGuyomar
Yannick Guyomar (YGuyomar) force-pushed the fix/cursor-globs-comma-join-3002 branch from 7bb0169 to c0c270a Compare September 22, 2026 08:28
Cursor's .mdc frontmatter never uses a YAML list for globs and never
shows quoted values in its own docs, but the Cursor converter emitted
a quoted YAML list for comma-separated applyTo (inherited from the
ensure_ascii=True, escaping non-ASCII description text as \uXXXX.

Add yaml_plain_scalar() (patterns.py) for description: prefer a bare
scalar verified via YAML's own resolver, fall back to single-quoted,
and only use a double-quoted UTF-8-preserving scalar for values
neither can represent (embedded newlines).

globs gets its own, more permissive yaml_globs_scalar(): always bare,
including patterns starting with "**". A leading "**" is technically
a YAML alias indicator under a strict parser, but Cursor's own docs
never quote it either and its frontmatter reader tolerates it -- glob
syntax has none of the ": "/reserved-word ambiguity that description
free text can have. The only thing still quoted is a literal embedded
newline, which would inject extra lines into the frontmatter block
regardless of parser leniency.

Wire both into _convert_to_cursor_rules; globs is always comma-joined
into one scalar instead of a YAML list. Scoped to Cursor only; other
targets keep yaml_double_quote().

Address review feedback from Copilot's automated PR review:

- Re-escape each glob (escape_apply_to_segment, renamed public) before
  comma-joining, so a literal comma the author escaped in applyTo
  (\,) doesn't become indistinguishable from the join separator.
- Route the plain/single-quoted round-trip check through the
  project's bounded YAML loader (load_yaml_str) instead of stock
  yaml.safe_load, closing a YAML alias/merge-key expansion DoS that
  parsing untrusted description text a second time would otherwise
  reintroduce.
- Escape DEL, NEL, LS, and PS, plus any lone UTF-16 surrogate, in the
  double-quoted fallback: ensure_ascii=False otherwise leaves them
  raw, which either changes silently on a strict YAML re-parse (NEL/
  LS/PS are YAML 1.1 line breaks) or crashes the eventual UTF-8 file
  write outright (DEL and lone surrogates cannot be UTF-8 encoded --
  the latter is this project's existing hidden-unicode security-gate
  attack shape).
- Update package-authoring.md's applyTo/globs section: Cursor's globs
  is a comma-joined bare scalar, not a YAML array like the other
  targets, and a literal comma inside one glob still isn't
  representable in a Cursor rule even though the escape round-trips
  through APM's own tooling.

Fixes microsoft#3002.
@YGuyomar
Yannick Guyomar (YGuyomar) force-pushed the fix/cursor-globs-comma-join-3002 branch from c0c270a to a1ecda1 Compare September 22, 2026 15:40

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/multi-target Multi-target deploy spec, target directory creation, agent surface routing. status/accepted Human scope approval; verify the issue's approval record and review contact before work. theme/portability One manifest, every target. Multi-target deploy, marketplace, packaging, install. triage/recommended Automated advice completed; not human scope approval. type/bug Something does not work as documented.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Cursor target emits quoted YAML list for globs instead of unquoted comma-separated string

3 participants