diff --git a/src/apm_cli/commands/compile/cli.py b/src/apm_cli/commands/compile/cli.py index 83dae74d..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: @@ -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 ca670abd..f06fc192 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -253,6 +253,9 @@ def _compile_distributed(self, config: CompilationConfig, primitives: PrimitiveC """ from .distributed_compiler import DistributedAgentsCompiler + errors = self.validate_primitives(primitives) + self.errors.extend(errors) + # Create distributed compiler with exclude patterns distributed_compiler = DistributedAgentsCompiler( str(self.base_dir), @@ -317,8 +320,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 ) @@ -405,6 +408,9 @@ def _compile_claude_md(self, config: CompilationConfig, primitives: PrimitiveCol Returns: CompilationResult: Result of the CLAUDE.md compilation. """ + errors = self.validate_primitives(primitives) + self.errors.extend(errors) + # Create Claude formatter claude_formatter = ClaudeFormatter(str(self.base_dir)) @@ -435,8 +441,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/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 2d18fab7..fee27f4c 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", target="agents" + ) + + 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 @@ -319,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/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) 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"),