fix: warn when instruction is missing required fields during `apm compile` by alopezsanchez · Pull Request #449 · microsoft/apm

alopezsanchez · 2026-03-25T16:57:59Z

Description

Instructions without applyTo were silently dropped during apm compile in distributed and claude modes because validate_primitives() was only called in the single-file path. This adds validation to all compilation modes and surfaces warnings through the CLI.

Call validate_primitives() in _compile_distributed() and _compile_claude_md()
Merge self.warnings into dry-run and normal compilation results
Remove distributed-mode warning suppression in CLI
Add unit and CLI integration tests for the warning

Type of change

Bug fix
New feature
Documentation
Maintenance / refactor

Testing

Tested locally
All existing tests pass
Added tests for new functionality (if applicable)

…ompile Instructions without applyTo were silently dropped during 'apm compile' in distributed and claude modes because validate_primitives() was only called in the single-file path. This adds validation to all compilation modes and surfaces warnings through the CLI. - Call validate_primitives() in _compile_distributed() and _compile_claude_md() - Merge self.warnings into dry-run and normal compilation results - Remove distributed-mode warning suppression in CLI - Add unit and CLI integration tests for the warning

alopezsanchez · 2026-03-25T16:59:51Z

src/apm_cli/commands/compile/cli.py


-        # Common error handling for both compilation modes
-        # Note: Warnings are handled by professional formatters for distributed mode
-        if config.strategy != "distributed" or single_agents:


I'm not sure about this line, but it took me some time to realize that I was missing the applyTo field. The apm compile wasn't adding the instruction to the AGENTS/CLAUDE.md, and I didn't know why.

Copilot

Pull request overview

Fixes missing validation coverage in apm compile so instructions missing required fields (notably applyTo) are no longer silently dropped in distributed/claude compilation paths, and ensures warnings are surfaced through the CLI.

Changes:

Run validate_primitives() in distributed and CLAUDE.md compilation paths.
Merge compiler warnings/errors into compilation results (including dry-run).
Add unit + CLI integration tests asserting warnings appear when applyTo is missing.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
tests/unit/compilation/test_compile_target_flag.py	Adds CLI-level tests that `apm compile` emits warnings for missing `applyTo` in distributed and claude targets.
tests/unit/compilation/test_compilation.py	Adds unit tests ensuring validation warnings propagate through distributed + claude compilation results.
src/apm_cli/compilation/agents_compiler.py	Invokes validation in more compilation modes and merges `self.warnings/self.errors` into results.
src/apm_cli/commands/compile/cli.py	Always prints warnings regardless of compilation strategy.

src/apm_cli/compilation/agents_compiler.py

tests/unit/compilation/test_compilation.py

Copilot · 2026-03-25T17:04:55Z

tests/unit/compilation/test_compile_target_flag.py

+        original_dir = os.getcwd()
+        try:
+            os.chdir(project_with_bad_instruction)
+            result = runner.invoke(
+                cli, ["compile", "--target", "vscode", "--dry-run"]
+            )


These CLI tests mutate the process working directory, which is easy to get wrong and makes tests harder to compose. Prefer pytest’s monkeypatch.chdir(project_with_bad_instruction) (or CliRunner.isolated_filesystem() plus constructing the project inside it) to keep the cwd change scoped and automatically restored.

Copilot · 2026-03-25T17:04:55Z

tests/unit/compilation/test_compile_target_flag.py

+        self, runner, project_with_bad_instruction
+    ):
+        """Test that apm compile --dry-run warns about missing applyTo in distributed mode."""
+        original_dir = os.getcwd()
+        try:
+            os.chdir(project_with_bad_instruction)
+            result = runner.invoke(
+                cli, ["compile", "--target", "vscode", "--dry-run"]
+            )
+            assert "applyTo" in result.output, (
+                f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
+            )
+        finally:
+            os.chdir(original_dir)
+
+    def test_cli_warns_missing_apply_to_claude(
+        self, runner, project_with_bad_instruction
+    ):
+        """Test that apm compile --target claude --dry-run warns about missing applyTo."""
+        original_dir = os.getcwd()
+        try:
+            os.chdir(project_with_bad_instruction)
+            result = runner.invoke(
+                cli, ["compile", "--target", "claude", "--dry-run"]
+            )
+            assert "applyTo" in result.output, (
+                f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"
+            )
+        finally:
+            os.chdir(original_dir)


These CLI tests mutate the process working directory, which is easy to get wrong and makes tests harder to compose. Prefer pytest’s monkeypatch.chdir(project_with_bad_instruction) (or CliRunner.isolated_filesystem() plus constructing the project inside it) to keep the cwd change scoped and automatically restored.

Suggested change

self, runner, project_with_bad_instruction

):

"""Test that apm compile --dry-run warns about missing applyTo in distributed mode."""

original_dir = os.getcwd()

try:

os.chdir(project_with_bad_instruction)

result = runner.invoke(

cli, ["compile", "--target", "vscode", "--dry-run"]

)

assert "applyTo" in result.output, (

f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"

)

finally:

os.chdir(original_dir)

def test_cli_warns_missing_apply_to_claude(

self, runner, project_with_bad_instruction

):

"""Test that apm compile --target claude --dry-run warns about missing applyTo."""

original_dir = os.getcwd()

try:

os.chdir(project_with_bad_instruction)

result = runner.invoke(

cli, ["compile", "--target", "claude", "--dry-run"]

)

assert "applyTo" in result.output, (

f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"

)

finally:

os.chdir(original_dir)

self, runner, project_with_bad_instruction, monkeypatch

):

"""Test that apm compile --dry-run warns about missing applyTo in distributed mode."""

monkeypatch.chdir(project_with_bad_instruction)

result = runner.invoke(cli, ["compile", "--target", "vscode", "--dry-run"])

assert "applyTo" in result.output, (

f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"

)

def test_cli_warns_missing_apply_to_claude(

self, runner, project_with_bad_instruction, monkeypatch

):

"""Test that apm compile --target claude --dry-run warns about missing applyTo."""

monkeypatch.chdir(project_with_bad_instruction)

result = runner.invoke(cli, ["compile", "--target", "claude", "--dry-run"])

assert "applyTo" in result.output, (

f"Expected warning about missing 'applyTo' in CLI output, got:\n{result.output}"

)

danielmeppiel · 2026-04-02T12:08:57Z

@alopezsanchez happy to merge after we address the review comments, thank you very much! Will also need to ensure compatibility with claude's "paths" which is equivalent to "applyTo".

…pile paths Address Copilot PR review comments: - Capture return value of validate_primitives() and extend self.errors in _compile_distributed() and _compile_claude_md(), matching the pattern already used in _compile_single_file() - Add target='agents' to test config to isolate distributed path testing

…note Instructions without applyTo are valid -- they apply globally: - Distributed compile: placed at project root - Claude deploy: becomes unconditional rule (no paths: key) - Copilot/Cursor: deployed verbatim The warning message now reflects this ('will apply globally') instead of implying applyTo is required. Suggestion updated to explain the choice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

danielmeppiel · 2026-04-06T22:16:51Z

Thanks @alopezsanchez -- the approach of surfacing validate_primitives() across all compile paths is correct.

I pushed one change to your branch: the warning message for missing applyTo was changed from:

Missing 'applyTo' in frontmatter (required for instructions)

to:

No 'applyTo' pattern specified -- instruction will apply globally

Rationale: applyTo is semantically optional, not required. Instructions without it work correctly across all paths:

Distributed compile: placed at project root directory
Claude deploy: becomes an unconditional rule (no paths: key emitted)
Copilot/Cursor: deployed verbatim, the tool decides scoping

The old message implied applyTo was mandatory, which would confuse users who intentionally write global instructions. The new message is informational -- it tells users what will happen rather than suggesting something is wrong.

Also updated the suggestion helper to offer both options: add applyTo for scoping, or leave as-is for global.

alopezsanchez requested a review from danielmeppiel as a code owner March 25, 2026 16:58

Copilot AI review requested due to automatic review settings March 25, 2026 16:58

alopezsanchez commented Mar 25, 2026

View reviewed changes

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Copilot started reviewing on behalf of alopezsanchez March 25, 2026 17:25 View session

danielmeppiel and others added 5 commits April 2, 2026 14:09

Merge branch 'main' into fix/warn-missing-applyto-on-compile

562f615

Merge branch 'microsoft:main' into fix/warn-missing-applyto-on-compile

d345622

Merge branch 'main' into fix/warn-missing-applyto-on-compile

65e8877

danielmeppiel merged commit 1288b0a into microsoft:main Apr 6, 2026
5 checks passed

This was referenced Apr 6, 2026

chore: prepare v0.8.11 release #598

Merged

[docs] Update documentation for features from 2026-04-07 #601

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: warn when instruction is missing required fields during `apm compile`#449

fix: warn when instruction is missing required fields during `apm compile`#449
danielmeppiel merged 6 commits intomicrosoft:mainfrom
alopezsanchez:fix/warn-missing-applyto-on-compile

alopezsanchez commented Mar 25, 2026 •

edited

Loading

Uh oh!

alopezsanchez Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

danielmeppiel commented Apr 2, 2026

Uh oh!

danielmeppiel commented Apr 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alopezsanchez commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Testing

Uh oh!

alopezsanchez Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

danielmeppiel commented Apr 2, 2026

Uh oh!

danielmeppiel commented Apr 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alopezsanchez commented Mar 25, 2026 •

edited

Loading