diff --git a/CHANGELOG.md b/CHANGELOG.md index f16280684..234634bb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Deduplicate Claude Code instructions.** `apm compile --target claude` now omits the "Project Standards" section from `CLAUDE.md` when instructions are already deployed to `.claude/rules/` by `apm install`, avoiding duplicate content in Claude Code's context window. `CLAUDE.md` is still generated for constitution and dependency imports. (#1138) - `apm install` no longer silently overwrites pre-existing governance files; `check_collision()` now treats `managed_files=None` (first install, no lockfile) as an empty set so hand-rolled files in `.github/instructions/` are detected and protected. (#1256) - Policy inheritance now fails closed: child policies that omit `unmanaged_files` inherit the parent's action instead of silently defaulting to `ignore`. (#1253) - MCP server token injection now requires both an allowlisted server name and a verified HTTPS GitHub hostname, preventing PAT exfiltration via poisoned registry entries. (#1239) diff --git a/docs/src/content/docs/producer/compile.md b/docs/src/content/docs/producer/compile.md index b739c296c..0ed182083 100644 --- a/docs/src/content/docs/producer/compile.md +++ b/docs/src/content/docs/producer/compile.md @@ -115,7 +115,7 @@ Per target, with the rules shape on disk after compile: | Target | Root context file | Per-rule output | Compile required? | |---|---|---|---| | `copilot` | `AGENTS.md` | `.github/instructions/.instructions.md` (preserves `applyTo`) | No -- Copilot reads the per-rule files natively | -| `claude` | `CLAUDE.md` | `.claude/rules/.md` | Yes -- `CLAUDE.md` is the entry point | +| `claude` | `CLAUDE.md` | `.claude/rules/.md` | Conditional -- needed for constitution/dependency imports; skipped when `.claude/rules/` already has instructions (see [deduplication note](#claude-code-deduplication)) | | `cursor` | -- | `.cursor/rules/.mdc` | Yes -- `.mdc` is Cursor's rules format | | `codex` | `AGENTS.md` (folded) | none -- compile-only, no per-file deploy | Yes -- folded into `AGENTS.md` | | `gemini` | `GEMINI.md` (folded) | none -- compile-only, no per-file deploy | Yes -- folded into `GEMINI.md` | @@ -137,6 +137,16 @@ correct AGENTS.md / CLAUDE.md / GEMINI.md output. Reach for `apm compile` directly when you are iterating on instructions and do not want install's side effects. +:::note[Claude Code deduplication] +When `apm install` has already deployed instructions to +`.claude/rules/`, `apm compile --target claude` automatically omits +the instructions section from `CLAUDE.md` to avoid duplicate content +in Claude Code's context window. `CLAUDE.md` is still generated when +it carries a constitution or dependency `@import` paths. If +`.claude/rules/` is later removed, re-running `apm compile` restores +the instructions section to `CLAUDE.md`. +::: + ## Pitfalls - **Confusing compile's scope.** Compile only handles **instructions** diff --git a/src/apm_cli/compilation/agents_compiler.py b/src/apm_cli/compilation/agents_compiler.py index 40ce31b83..adcc758e1 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -552,8 +552,39 @@ def _compile_claude_md( debug=config.debug, ) + # Skip instructions in CLAUDE.md when they are already deployed to + # .claude/rules/ by `apm install` (avoids duplicate context in Claude Code). + claude_rules_dir = self.base_dir / ".claude" / "rules" + skip_instructions = False + if claude_rules_dir.is_dir(): + from ..utils.path_security import PathTraversalError, ensure_path_within + + try: + ensure_path_within(claude_rules_dir, self.base_dir) + except PathTraversalError: + self._log( + "progress", + ".claude/rules/ is a symlink outside the project root -- ignoring", + symbol="warning", + ) + else: + if any(claude_rules_dir.glob("*.md")): + skip_instructions = True + + if skip_instructions: + self._log( + "progress", + "Instructions already in .claude/rules/ -- omitting from CLAUDE.md" + " to avoid duplicate context", + symbol="info", + ) + # Format CLAUDE.md files - claude_config = {"source_attribution": config.source_attribution, "debug": config.debug} + claude_config = { + "source_attribution": config.source_attribution, + "debug": config.debug, + "skip_instructions": skip_instructions, + } claude_result = claude_formatter.format_distributed( primitives, placement_map, claude_config ) @@ -568,8 +599,9 @@ def _compile_claude_md( # Handle dry-run mode if config.dry_run: # Generate preview summary + count = len(claude_result.placements) preview_lines = [ - f"CLAUDE.md Preview: Would generate {len(claude_result.placements)} files" + f"CLAUDE.md Preview: Would generate {count} {'file' if count == 1 else 'files'}" ] for claude_path in claude_result.content_map.keys(): # noqa: SIM118 rel_path = portable_relpath(claude_path, self.base_dir) @@ -628,15 +660,25 @@ def _compile_claude_md( stats = claude_result.stats.copy() stats["claude_files_written"] = files_written + if files_written == 0 and skip_instructions: + self._log( + "progress", + "CLAUDE.md not generated -- Claude Code reads .claude/rules/ directly," + " no further action needed", + symbol="info", + ) + # Display CLAUDE.md compilation output using standard formatter # Get proper compilation results from distributed compiler (has optimization decisions) + # Skip formatter output when deduplication filtered out all placements to + # avoid contradicting the "not generated" log message above. from ..output.formatters import CompilationFormatter from ..output.models import CompilationResults compilation_results = distributed_compiler.get_compilation_results_for_display( is_dry_run=config.dry_run ) - if compilation_results: + if compilation_results and not (skip_instructions and files_written == 0): # Update target name for CLAUDE.md output formatter_results = CompilationResults( project_analysis=compilation_results.project_analysis, diff --git a/src/apm_cli/compilation/claude_formatter.py b/src/apm_cli/compilation/claude_formatter.py index 3d6959a6b..e37dd3cb1 100644 --- a/src/apm_cli/compilation/claude_formatter.py +++ b/src/apm_cli/compilation/claude_formatter.py @@ -37,6 +37,7 @@ class ClaudePlacement: dependencies: builtins.list[str] = field(default_factory=list) # @import paths coverage_patterns: builtins.set[str] = field(default_factory=set) source_attribution: builtins.dict[str, str] = field(default_factory=dict) + is_root: bool = False @dataclass @@ -98,24 +99,40 @@ def format_distributed( try: config = config or {} source_attribution = config.get("source_attribution", True) + skip_instructions = config.get("skip_instructions", False) # Generate Claude placements from the placement map placements = self._generate_placements( placement_map, primitives, source_attribution=source_attribution ) - # Generate content for each placement + # Generate content for each placement. + # When instructions are skipped (already in .claude/rules/), only + # emit root CLAUDE.md if it has other content (constitution or + # dependencies); subdirectory placements are omitted entirely. + has_constitution = bool(read_constitution(self.base_dir)) content_map = {} for placement in placements: - content = self._generate_claude_content(placement, primitives) + if skip_instructions: + if not placement.is_root: + continue + if not placement.dependencies and not has_constitution: + continue + content = self._generate_claude_content( + placement, primitives, skip_instructions=skip_instructions + ) content_map[placement.claude_path] = content + # Filter placements to only those that produced content so stats + # and downstream consumers see an accurate picture. + emitted_placements = [p for p in placements if p.claude_path in content_map] + # Compile statistics - stats = self._compile_stats(placements, primitives) + stats = self._compile_stats(emitted_placements, primitives) return ClaudeCompilationResult( success=len(self.errors) == 0, - placements=placements, + placements=emitted_placements, content_map=content_map, warnings=self.warnings.copy(), errors=self.errors.copy(), @@ -151,19 +168,20 @@ def _generate_placements( """ placements = [] - # Handle empty placement map with constitution + # Handle empty placement map with constitution or dependencies if not placement_map: constitution = read_constitution(self.base_dir) - if constitution: - # Create root placement for constitution-only projects + dependencies = self._collect_dependencies() + if constitution or dependencies: root_path = self.base_dir / "CLAUDE.md" placement = ClaudePlacement( claude_path=root_path, instructions=[], agents=list(primitives.chatmodes), - dependencies=self._collect_dependencies(), + dependencies=dependencies, coverage_patterns=set(), source_attribution={}, + is_root=True, ) placements.append(placement) else: @@ -197,6 +215,7 @@ def _generate_placements( dependencies=self._collect_dependencies() if is_root else [], coverage_patterns=patterns, source_attribution=source_map, + is_root=is_root, ) placements.append(placement) @@ -236,13 +255,18 @@ def _collect_dependencies(self) -> builtins.list[str]: return sorted(dependencies) def _generate_claude_content( - self, placement: ClaudePlacement, primitives: PrimitiveCollection + self, + placement: ClaudePlacement, + primitives: PrimitiveCollection, + *, + skip_instructions: bool = False, ) -> str: """Generate CLAUDE.md content for a specific placement. Args: placement (ClaudePlacement): Placement result with instructions. primitives (PrimitiveCollection): Full primitive collection. + skip_instructions (bool): If True, omit the Project Standards section. Returns: str: Generated CLAUDE.md content. @@ -264,8 +288,7 @@ def _generate_claude_content( sections.append("") # Constitution section (only for root CLAUDE.md) - is_root = placement.claude_path.parent == self.base_dir - if is_root: + if placement.is_root: constitution = read_constitution(self.base_dir) if constitution: sections.append("# Constitution") @@ -273,8 +296,11 @@ def _generate_claude_content( sections.append(constitution.strip()) sections.append("") - # Project Standards section (grouped by pattern) - if placement.instructions: + # Project Standards section (grouped by pattern). + # Skipped when instructions are already deployed to .claude/rules/ by + # `apm install`, since Claude Code reads both locations and would see + # duplicate content. + if placement.instructions and not skip_instructions: sections.append("# Project Standards") sections.append("") @@ -296,9 +322,7 @@ def _emit(instruction: Instruction) -> builtins.list[str]: ) ) - # Note: CLAUDE.md only contains instructions (Project Standards). - # Agents/workflows are NOT included - they go to .github/agents/ as separate files. - # This matches AGENTS.md behavior which also only contains instructions. + # Agents/workflows go to .github/agents/ as separate files, not here. # Footer sections.append("---") diff --git a/tests/unit/compilation/test_agents_compiler_coverage.py b/tests/unit/compilation/test_agents_compiler_coverage.py index 23fb8c3ec..1b2cbc5a1 100644 --- a/tests/unit/compilation/test_agents_compiler_coverage.py +++ b/tests/unit/compilation/test_agents_compiler_coverage.py @@ -671,5 +671,180 @@ def test_compile_claude_md_constitution_injection_failure(self): ) +# --------------------------------------------------------------------------- +# AgentsCompiler._compile_claude_md() -- skip_instructions when .claude/rules/ populated +# --------------------------------------------------------------------------- + + +class TestClaudeCompileSkipInstructions(unittest.TestCase): + """Test that _compile_claude_md skips instructions when .claude/rules/ has files.""" + + def setUp(self): + self.original_dir = os.getcwd() + self.tmp = tempfile.mkdtemp() + self.tmp_resolved = str(Path(self.tmp).resolve()) + os.chdir(self.tmp_resolved) + # Minimal apm.yml so discovery works + with open("apm.yml", "w") as f: + yaml.dump({"name": "test-project", "version": "1.0.0"}, f) + # Create .apm/instructions with a sample instruction + instr_dir = Path(self.tmp_resolved) / ".apm" / "instructions" + instr_dir.mkdir(parents=True) + (instr_dir / "style.instructions.md").write_text( + "---\ndescription: Style guide\napplyTo: '**/*.py'\n---\nUse type hints.\n" + ) + + def tearDown(self): + os.chdir(self.original_dir) + import shutil + + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_instructions_included_without_rules_dir(self): + """Without .claude/rules/, instructions appear in CLAUDE.md.""" + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=False) + result = compiler.compile(config) + self.assertTrue(result.success) + claude_md = Path(self.tmp_resolved) / "CLAUDE.md" + self.assertTrue(claude_md.exists()) + self.assertIn("# Project Standards", claude_md.read_text()) + + def test_instructions_skipped_with_populated_rules_dir(self): + """With .claude/rules/ containing .md files, instructions are skipped.""" + rules_dir = Path(self.tmp_resolved) / ".claude" / "rules" + rules_dir.mkdir(parents=True) + (rules_dir / "style.md").write_text("---\npaths:\n - '**/*.py'\n---\nUse type hints.\n") + + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=False) + result = compiler.compile(config) + self.assertTrue(result.success) + + # No constitution or dependencies, so CLAUDE.md should not be generated + claude_md = Path(self.tmp_resolved) / "CLAUDE.md" + self.assertFalse(claude_md.exists()) + + def test_instructions_not_skipped_with_empty_rules_dir(self): + """An empty .claude/rules/ does not trigger instruction skipping.""" + rules_dir = Path(self.tmp_resolved) / ".claude" / "rules" + rules_dir.mkdir(parents=True) + + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=False) + result = compiler.compile(config) + self.assertTrue(result.success) + + claude_md = Path(self.tmp_resolved) / "CLAUDE.md" + self.assertTrue(claude_md.exists()) + content = claude_md.read_text() + self.assertIn("# Project Standards", content) + + def test_instructions_not_skipped_with_non_md_files_in_rules_dir(self): + """Non-.md files in .claude/rules/ do not trigger instruction skipping.""" + rules_dir = Path(self.tmp_resolved) / ".claude" / "rules" + rules_dir.mkdir(parents=True) + (rules_dir / ".gitkeep").write_text("") + (rules_dir / "notes.txt").write_text("some notes") + + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=False) + result = compiler.compile(config) + self.assertTrue(result.success) + + claude_md = Path(self.tmp_resolved) / "CLAUDE.md" + self.assertTrue(claude_md.exists()) + content = claude_md.read_text() + self.assertIn("# Project Standards", content) + + def test_skip_instructions_dry_run(self): + """Dry-run respects skip_instructions when .claude/rules/ is populated.""" + rules_dir = Path(self.tmp_resolved) / ".claude" / "rules" + rules_dir.mkdir(parents=True) + (rules_dir / "style.md").write_text("---\npaths:\n - '**/*.py'\n---\nUse type hints.\n") + + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=True) + result = compiler.compile(config) + self.assertTrue(result.success) + self.assertEqual(result.stats.get("claude_files_generated", 0), 0) + self.assertIn("Would generate 0 files", result.content) + + def test_dry_run_reports_file_without_skip(self): + """Dry-run without .claude/rules/ reports CLAUDE.md would be generated.""" + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=True) + result = compiler.compile(config) + self.assertTrue(result.success) + self.assertIn("Would generate 1 file", result.content) + self.assertIn("CLAUDE.md", result.content) + + def test_skip_instructions_stats_reflect_emitted_files(self): + """Stats report zero files generated when all placements are skipped.""" + rules_dir = Path(self.tmp_resolved) / ".claude" / "rules" + rules_dir.mkdir(parents=True) + (rules_dir / "style.md").write_text("---\npaths:\n - '**/*.py'\n---\nUse type hints.\n") + + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=True) + result = compiler.compile(config) + self.assertTrue(result.success) + self.assertEqual(result.stats.get("claude_files_generated", 0), 0) + + def test_skip_instructions_emits_log_messages(self): + """When instructions are skipped, informational log messages are emitted.""" + rules_dir = Path(self.tmp_resolved) / ".claude" / "rules" + rules_dir.mkdir(parents=True) + (rules_dir / "style.md").write_text("---\npaths:\n - '**/*.py'\n---\nUse type hints.\n") + + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=False) + + mock_logger = MagicMock() + result = compiler.compile(config, logger=mock_logger) + self.assertTrue(result.success) + + # Collect all progress messages + progress_calls = [ + str(call) for call in mock_logger.progress.call_args_list + ] + joined = " ".join(progress_calls) + self.assertIn("Instructions already in .claude/rules/", joined) + self.assertIn("CLAUDE.md not generated", joined) + + + def test_skip_instructions_ignores_symlink_outside_project(self): + """A .claude/rules/ symlinked outside the project does not trigger skip.""" + import shutil + + # Create a rules directory in a separate unique temp directory + external_dir = Path(tempfile.mkdtemp()) + try: + (external_dir / "style.md").write_text( + "---\npaths:\n - '**/*.py'\n---\nHacked.\n" + ) + + # Symlink .claude/rules/ to the external directory + claude_dir = Path(self.tmp_resolved) / ".claude" + claude_dir.mkdir(parents=True, exist_ok=True) + rules_link = claude_dir / "rules" + try: + rules_link.symlink_to(external_dir) + except OSError: + self.skipTest("symlinks not supported on this platform") + + compiler = AgentsCompiler(self.tmp_resolved) + config = CompilationConfig(target="claude", dry_run=False) + result = compiler.compile(config) + self.assertTrue(result.success) + + # Instructions should NOT be skipped (symlink escapes project root) + claude_md = Path(self.tmp_resolved) / "CLAUDE.md" + self.assertTrue(claude_md.exists()) + self.assertIn("# Project Standards", claude_md.read_text()) + finally: + shutil.rmtree(external_dir, ignore_errors=True) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/compilation/test_claude_formatter.py b/tests/unit/compilation/test_claude_formatter.py index af1363959..ee2264d99 100644 --- a/tests/unit/compilation/test_claude_formatter.py +++ b/tests/unit/compilation/test_claude_formatter.py @@ -496,3 +496,153 @@ def test_format_distributed_handles_exceptions(self, temp_project): result = formatter.format_distributed(primitives, {temp_project: [instruction]}) assert isinstance(result, ClaudeCompilationResult) + + +class TestSkipInstructions: + """Tests for skip_instructions behavior when .claude/rules/ is populated.""" + + @pytest.fixture + def temp_project(self): + """Create a temporary project directory.""" + temp_dir = tempfile.mkdtemp() + resolved = Path(temp_dir).resolve() + yield resolved + shutil.rmtree(resolved, ignore_errors=True) + + @pytest.fixture + def sample_primitives(self, temp_project): + """Create sample primitives for testing.""" + primitives = PrimitiveCollection() + instruction = Instruction( + name="python-style", + file_path=temp_project / ".apm/instructions/python.instructions.md", + description="Python coding standards", + apply_to="**/*.py", + content="Use type hints and follow PEP 8.", + author="test", + source="local", + ) + primitives.add_primitive(instruction) + return primitives + + def test_skip_instructions_omits_project_standards(self, temp_project, sample_primitives): + """When skip_instructions is True, Project Standards section is omitted.""" + formatter = ClaudeFormatter(str(temp_project)) + placement_map = {temp_project: list(sample_primitives.instructions)} + + config = {"skip_instructions": True} + result = formatter.format_distributed(sample_primitives, placement_map, config) + + # No files generated (no constitution or dependencies either) + assert result.success + assert len(result.content_map) == 0 + # Stats reflect zero emitted files, not the original placement count + assert result.stats["claude_files_generated"] == 0 + assert result.placements == [] + + def test_skip_instructions_preserves_constitution(self, temp_project, sample_primitives): + """When skip_instructions is True, constitution is still included.""" + from apm_cli.compilation.constitution import clear_constitution_cache + + # Create a constitution file at the expected path + memory_dir = temp_project / ".specify" / "memory" + memory_dir.mkdir(parents=True) + (memory_dir / "constitution.md").write_text("Always be helpful.") + + clear_constitution_cache() + formatter = ClaudeFormatter(str(temp_project)) + placement_map = {temp_project: list(sample_primitives.instructions)} + + config = {"skip_instructions": True} + result = formatter.format_distributed(sample_primitives, placement_map, config) + + assert result.success + assert len(result.content_map) == 1 + content = result.content_map[temp_project / "CLAUDE.md"] + assert "# Constitution" in content + assert "Always be helpful." in content + assert "# Project Standards" not in content + + def test_skip_instructions_preserves_dependencies(self, temp_project, sample_primitives): + """When skip_instructions is True, dependencies are still included.""" + # Create a dependency with CLAUDE.md + dep_dir = temp_project / "apm_modules" / "owner" / "package" + dep_dir.mkdir(parents=True) + (dep_dir / "CLAUDE.md").write_text("# dep") + + formatter = ClaudeFormatter(str(temp_project)) + placement_map = {temp_project: list(sample_primitives.instructions)} + + config = {"skip_instructions": True} + result = formatter.format_distributed(sample_primitives, placement_map, config) + + assert result.success + assert len(result.content_map) == 1 + content = result.content_map[temp_project / "CLAUDE.md"] + assert "# Dependencies" in content + assert "@apm_modules/owner/package/CLAUDE.md" in content + assert "# Project Standards" not in content + + def test_skip_instructions_omits_subdirectory_files(self, temp_project, sample_primitives): + """When skip_instructions is True, subdirectory CLAUDE.md files are not generated.""" + subdir = temp_project / "src" + subdir.mkdir() + + primitives = PrimitiveCollection() + instruction = Instruction( + name="src-style", + file_path=temp_project / ".apm/instructions/src.instructions.md", + description="Source standards", + apply_to="src/**/*.py", + content="Source-specific rules.", + author="test", + source="local", + ) + primitives.add_primitive(instruction) + + placement_map = {subdir: list(primitives.instructions)} + + config = {"skip_instructions": True} + formatter = ClaudeFormatter(str(temp_project)) + result = formatter.format_distributed(primitives, placement_map, config) + + assert result.success + assert len(result.content_map) == 0 + + def test_no_skip_instructions_includes_project_standards( + self, temp_project, sample_primitives + ): + """When skip_instructions is False (default), instructions are included.""" + formatter = ClaudeFormatter(str(temp_project)) + placement_map = {temp_project: list(sample_primitives.instructions)} + + config = {"skip_instructions": False} + result = formatter.format_distributed(sample_primitives, placement_map, config) + + assert result.success + assert len(result.content_map) == 1 + content = result.content_map[temp_project / "CLAUDE.md"] + assert "# Project Standards" in content + assert "Use type hints and follow PEP 8." in content + + def test_is_root_flag_used_for_skip_filtering(self, temp_project, sample_primitives): + """The is_root flag on ClaudePlacement drives skip filtering, not path comparison.""" + # Create a dependency so root CLAUDE.md is emitted even with skip + dep_dir = temp_project / "apm_modules" / "owner" / "package" + dep_dir.mkdir(parents=True) + (dep_dir / "CLAUDE.md").write_text("# dep") + + formatter = ClaudeFormatter(str(temp_project)) + placement_map = {temp_project: list(sample_primitives.instructions)} + config = {"skip_instructions": True} + + result = formatter.format_distributed(sample_primitives, placement_map, config) + + assert result.success + assert len(result.content_map) == 1 + # Root placement was emitted (has dependencies) + assert len(result.placements) == 1 + assert result.placements[0].is_root is True + content = next(iter(result.content_map.values())) + assert "# Dependencies" in content + assert "# Project Standards" not in content