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.

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

src/apm_cli/compilation/agents_compiler.py

        """
        from .distributed_compiler import DistributedAgentsCompiler

+        self.validate_primitives(primitives)


validate_primitives() is documented to return validation errors, and the single-file path captures/propagates those return values into self.errors. In _compile_distributed() / _compile_claude_md() the return value is ignored, so any future (or existing) validation logic that returns errors would be silently dropped for these modes. Capture the returned list and extend self.errors (matching _compile_single_file()), or alternatively update validate_primitives() to have a warning-only contract and return type to avoid API inconsistency.

Suggested change

self.validate_primitives(primitives)

validation_errors = self.validate_primitives(primitives)

self.errors.extend(validation_errors)

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

src/apm_cli/compilation/agents_compiler.py

        Returns:
            CompilationResult: Result of the CLAUDE.md compilation.
        """
+        self.validate_primitives(primitives)


validate_primitives() is documented to return validation errors, and the single-file path captures/propagates those return values into self.errors. In _compile_distributed() / _compile_claude_md() the return value is ignored, so any future (or existing) validation logic that returns errors would be silently dropped for these modes. Capture the returned list and extend self.errors (matching _compile_single_file()), or alternatively update validate_primitives() to have a warning-only contract and return type to avoid API inconsistency.

Suggested change

self.validate_primitives(primitives)

self.errors.extend(self.validate_primitives(primitives))

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

tests/unit/compilation/test_compilation.py

+
+        compiler = AgentsCompiler(str(self.temp_path))
+        config = CompilationConfig(
+            dry_run=True, resolve_links=False, strategy="distributed"


This test is named/scoped to “distributed compile”, but CompilationConfig defaults target="all" (per the provided context), so compiler.compile() will also run the CLAUDE.md compilation path and merge results. That makes the test less isolated and can mask which target actually produced the warning. Set target="vscode"/"agents" explicitly so the test exercises only the distributed AGENTS.md path.

Suggested change

dry_run=True, resolve_links=False, strategy="distributed"

dry_run=True,

resolve_links=False,

strategy="distributed",

target="agents",

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}"

)

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

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
alopezsanchez wants to merge 1 commit 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!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	self.validate_primitives(primitives)
	validation_errors = self.validate_primitives(primitives)
	self.errors.extend(validation_errors)

	self.validate_primitives(primitives)
	self.errors.extend(self.validate_primitives(primitives))

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!

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!

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!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alopezsanchez commented Mar 25, 2026 •

edited

Loading