From 7eb0a8510827db32b778a26a827e8159329768a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20L=C3=B3pez=20S=C3=A1nchez?= Date: Wed, 25 Mar 2026 17:09:41 +0100 Subject: [PATCH 1/3] fix: warn when instruction is missing required applyTo field during compile 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 --- src/apm_cli/commands/compile/cli.py | 17 ++-- src/apm_cli/compilation/agents_compiler.py | 12 ++- tests/unit/compilation/test_compilation.py | 84 +++++++++++++++++++ .../compilation/test_compile_target_flag.py | 60 +++++++++++++ 4 files changed, 159 insertions(+), 14 deletions(-) diff --git a/src/apm_cli/commands/compile/cli.py b/src/apm_cli/commands/compile/cli.py index e29f5a17..9eb125a3 100644 --- a/src/apm_cli/commands/compile/cli.py +++ b/src/apm_cli/commands/compile/cli.py @@ -529,16 +529,13 @@ def compile( else: _display_next_steps(output) - # Common error handling for both compilation modes - # Note: Warnings are handled by professional formatters for distributed mode - if config.strategy != "distributed" or single_agents: - # Only show warnings for single-file mode (backward compatibility) - if result.warnings: - logger.warning( - f"Compilation completed with {len(result.warnings)} warnings:" - ) - for warning in result.warnings: - logger.warning(f" {warning}") + # Display warnings for all compilation modes + if result.warnings: + logger.warning( + f"Compilation completed with {len(result.warnings)} warning(s):" + ) + for warning in result.warnings: + logger.warning(f" {warning}") if result.errors: logger.error(f"Compilation failed with {len(result.errors)} errors:") diff --git a/src/apm_cli/compilation/agents_compiler.py b/src/apm_cli/compilation/agents_compiler.py index a1619648..6cf19215 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -247,6 +247,8 @@ def _compile_distributed(self, config: CompilationConfig, primitives: PrimitiveC """ from .distributed_compiler import DistributedAgentsCompiler + self.validate_primitives(primitives) + # Create distributed compiler with exclude patterns distributed_compiler = DistributedAgentsCompiler( str(self.base_dir), @@ -311,8 +313,8 @@ def _compile_distributed(self, config: CompilationConfig, primitives: PrimitiveC success=True, output_path="Preview mode - no files written", content=self._generate_placement_summary(distributed_result), - warnings=distributed_result.warnings, - errors=distributed_result.errors, + warnings=self.warnings + distributed_result.warnings, + errors=self.errors + distributed_result.errors, stats=distributed_result.stats ) @@ -399,6 +401,8 @@ def _compile_claude_md(self, config: CompilationConfig, primitives: PrimitiveCol Returns: CompilationResult: Result of the CLAUDE.md compilation. """ + self.validate_primitives(primitives) + # Create Claude formatter claude_formatter = ClaudeFormatter(str(self.base_dir)) @@ -429,8 +433,8 @@ def _compile_claude_md(self, config: CompilationConfig, primitives: PrimitiveCol # not at compile time. This keeps behavior consistent with VSCode prompt integration. # Merge warnings and errors (no command result anymore) - all_warnings = claude_result.warnings - all_errors = claude_result.errors + all_warnings = self.warnings + claude_result.warnings + all_errors = self.errors + claude_result.errors # Handle dry-run mode if config.dry_run: diff --git a/tests/unit/compilation/test_compilation.py b/tests/unit/compilation/test_compilation.py index 2d18fab7..712e46b6 100644 --- a/tests/unit/compilation/test_compilation.py +++ b/tests/unit/compilation/test_compilation.py @@ -156,6 +156,29 @@ def test_validate_primitives(self): errors = compiler.validate_primitives(primitives) self.assertEqual(len(errors), 0) + def test_validate_primitives_warns_on_missing_apply_to(self): + """Test that validate_primitives adds a warning when applyTo is missing.""" + compiler = AgentsCompiler(str(self.temp_path)) + + primitives = PrimitiveCollection() + instruction = Instruction( + name="no-scope", + file_path=self.temp_path / "no-scope.instructions.md", + description="Instruction without applyTo", + apply_to="", + content="Some content", + author="test", + ) + primitives.add_primitive(instruction) + + errors = compiler.validate_primitives(primitives) + self.assertEqual(len(errors), 0) + self.assertTrue(len(compiler.warnings) > 0) + self.assertTrue( + any("applyTo" in w for w in compiler.warnings), + f"Expected a warning mentioning 'applyTo', got: {compiler.warnings}", + ) + @patch('apm_cli.primitives.discovery.discover_primitives') def test_compile_with_mock_primitives(self, mock_discover): """Test compilation with mocked primitives.""" @@ -185,6 +208,67 @@ def test_compile_with_mock_primitives(self, mock_discover): self.assertIn("Files matching `**/*.py`", result.content) self.assertIn("Use type hints.", result.content) + def test_distributed_compile_includes_validation_warnings(self): + """Test that distributed compilation surfaces warnings for missing applyTo.""" + primitives = PrimitiveCollection() + + good_instruction = Instruction( + name="good", + file_path=self.temp_path / "good.instructions.md", + description="Valid instruction", + apply_to="**/*.py", + content="Follow PEP 8.", + author="test", + ) + bad_instruction = Instruction( + name="bad", + file_path=self.temp_path / "bad.instructions.md", + description="Missing applyTo", + apply_to="", + content="This has no scope.", + author="test", + ) + primitives.add_primitive(good_instruction) + primitives.add_primitive(bad_instruction) + + compiler = AgentsCompiler(str(self.temp_path)) + config = CompilationConfig( + dry_run=True, resolve_links=False, strategy="distributed" + ) + + result = compiler.compile(config, primitives) + + self.assertTrue( + any("applyTo" in w for w in result.warnings), + f"Expected a warning about missing 'applyTo', got: {result.warnings}", + ) + + def test_claude_md_compile_includes_validation_warnings(self): + """Test that CLAUDE.md compilation surfaces warnings for missing applyTo.""" + primitives = PrimitiveCollection() + + bad_instruction = Instruction( + name="no-scope", + file_path=self.temp_path / "no-scope.instructions.md", + description="Missing applyTo", + apply_to="", + content="This has no scope.", + author="test", + ) + primitives.add_primitive(bad_instruction) + + compiler = AgentsCompiler(str(self.temp_path)) + config = CompilationConfig( + dry_run=True, resolve_links=False, target="claude" + ) + + result = compiler.compile(config, primitives) + + self.assertTrue( + any("applyTo" in w for w in result.warnings), + f"Expected a warning about missing 'applyTo', got: {result.warnings}", + ) + def test_compile_agents_md_function(self): """Test the standalone compile function.""" # Create test primitives diff --git a/tests/unit/compilation/test_compile_target_flag.py b/tests/unit/compilation/test_compile_target_flag.py index 7178d124..01df31c3 100644 --- a/tests/unit/compilation/test_compile_target_flag.py +++ b/tests/unit/compilation/test_compile_target_flag.py @@ -847,3 +847,63 @@ def test_cli_override_takes_precedence(self, temp_project_with_config): assert config.target == "vscode" finally: os.chdir(original_dir) + + +class TestCompileWarningOnMissingApplyTo: + """Tests that apm compile warns when an instruction is missing applyTo.""" + + @pytest.fixture + def runner(self): + return CliRunner() + + @pytest.fixture + def project_with_bad_instruction(self): + temp_dir = tempfile.mkdtemp() + temp_path = Path(temp_dir) + + (temp_path / "apm.yml").write_text("name: test-project\nversion: 0.1.0\n") + + apm_dir = temp_path / ".apm" / "instructions" + apm_dir.mkdir(parents=True) + + (apm_dir / "good.instructions.md").write_text( + "---\napplyTo: '**/*.py'\n---\nFollow PEP 8.\n" + ) + (apm_dir / "bad.instructions.md").write_text( + "---\ndescription: Missing applyTo\n---\nThis instruction has no scope.\n" + ) + + yield temp_path + shutil.rmtree(temp_dir, ignore_errors=True) + + def test_cli_warns_missing_apply_to_distributed( + 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) From e45a9d7c71b3e58bb827343237546ccaa1af56c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20L=C3=B3pez=20S=C3=A1nchez?= Date: Mon, 6 Apr 2026 15:34:45 +0200 Subject: [PATCH 2/3] fix: capture validate_primitives errors in distributed and claude compile 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 --- src/apm_cli/compilation/agents_compiler.py | 6 ++++-- tests/unit/compilation/test_compilation.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/apm_cli/compilation/agents_compiler.py b/src/apm_cli/compilation/agents_compiler.py index 6cf19215..b54de280 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -247,7 +247,8 @@ def _compile_distributed(self, config: CompilationConfig, primitives: PrimitiveC """ from .distributed_compiler import DistributedAgentsCompiler - self.validate_primitives(primitives) + errors = self.validate_primitives(primitives) + self.errors.extend(errors) # Create distributed compiler with exclude patterns distributed_compiler = DistributedAgentsCompiler( @@ -401,7 +402,8 @@ def _compile_claude_md(self, config: CompilationConfig, primitives: PrimitiveCol Returns: CompilationResult: Result of the CLAUDE.md compilation. """ - self.validate_primitives(primitives) + errors = self.validate_primitives(primitives) + self.errors.extend(errors) # Create Claude formatter claude_formatter = ClaudeFormatter(str(self.base_dir)) diff --git a/tests/unit/compilation/test_compilation.py b/tests/unit/compilation/test_compilation.py index 712e46b6..f6c3c921 100644 --- a/tests/unit/compilation/test_compilation.py +++ b/tests/unit/compilation/test_compilation.py @@ -233,7 +233,7 @@ def test_distributed_compile_includes_validation_warnings(self): compiler = AgentsCompiler(str(self.temp_path)) config = CompilationConfig( - dry_run=True, resolve_links=False, strategy="distributed" + dry_run=True, resolve_links=False, strategy="distributed", target="agents" ) result = compiler.compile(config, primitives) From 455d823fa898e543e996384168d87ad43c15f24e Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 7 Apr 2026 00:16:36 +0200 Subject: [PATCH 3/3] fix: change missing applyTo from error-like warning to informational 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> --- src/apm_cli/commands/compile/cli.py | 4 ++-- src/apm_cli/primitives/models.py | 2 +- tests/unit/compilation/test_compilation.py | 4 ++-- tests/unit/primitives/test_primitives.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/apm_cli/commands/compile/cli.py b/src/apm_cli/commands/compile/cli.py index 7aef83d6..a0944065 100644 --- a/src/apm_cli/commands/compile/cli.py +++ b/src/apm_cli/commands/compile/cli.py @@ -154,8 +154,8 @@ def _get_validation_suggestion(error_msg): """Get actionable suggestions for validation errors.""" if "Missing 'description'" in error_msg: return "Add 'description: Your description here' to frontmatter" - elif "Missing 'applyTo'" in error_msg: - return "Add 'applyTo: \"**/*.py\"' to frontmatter" + elif "applyTo" in error_msg and "globally" in error_msg: + return "Add 'applyTo: \"**/*.py\"' to scope the instruction, or leave as-is for global" elif "Empty content" in error_msg: return "Add markdown content below the frontmatter" else: diff --git a/src/apm_cli/primitives/models.py b/src/apm_cli/primitives/models.py index 857d2ae1..cee7d76d 100644 --- a/src/apm_cli/primitives/models.py +++ b/src/apm_cli/primitives/models.py @@ -53,7 +53,7 @@ def validate(self) -> List[str]: if not self.description: errors.append("Missing 'description' in frontmatter") if not self.apply_to: - errors.append("Missing 'applyTo' in frontmatter (required for instructions)") + errors.append("No 'applyTo' pattern specified -- instruction will apply globally") if not self.content.strip(): errors.append("Empty content") return errors diff --git a/tests/unit/compilation/test_compilation.py b/tests/unit/compilation/test_compilation.py index f6c3c921..fee27f4c 100644 --- a/tests/unit/compilation/test_compilation.py +++ b/tests/unit/compilation/test_compilation.py @@ -403,8 +403,8 @@ def test_validate_mode_with_valid_primitives(self): suggestion = _get_validation_suggestion("Missing 'description' in frontmatter") self.assertIn("Add 'description:", suggestion) - suggestion = _get_validation_suggestion("Missing 'applyTo' in frontmatter") - self.assertIn("Add 'applyTo:", suggestion) + suggestion = _get_validation_suggestion("No 'applyTo' pattern specified -- instruction will apply globally") + self.assertIn("applyTo", suggestion) suggestion = _get_validation_suggestion("Empty content") self.assertIn("Add markdown content", suggestion) diff --git a/tests/unit/primitives/test_primitives.py b/tests/unit/primitives/test_primitives.py index e4cc3c0b..1bc7c2d7 100644 --- a/tests/unit/primitives/test_primitives.py +++ b/tests/unit/primitives/test_primitives.py @@ -73,7 +73,7 @@ def test_instruction_validation(self): ) self.assertEqual(instruction.validate(), []) - # Missing applyTo (required for instructions) + # Missing applyTo (instruction will apply globally) instruction_no_apply = Instruction( name="test", file_path=Path("test.instructions.md"),