diff --git a/.apm/instructions/architecture.instructions.md b/.apm/instructions/architecture.instructions.md index fb8ad35e2..8c0174768 100644 --- a/.apm/instructions/architecture.instructions.md +++ b/.apm/instructions/architecture.instructions.md @@ -46,6 +46,7 @@ semicolon-delimited, and specific to the file(s) that own the fact. | Runtime descriptors | runtime/registry.py | `src/apm_cli/runtime/registry.py` | | User-facing output / diagnostics | CommandLogger / console owner | `src/apm_cli/core/command_logger.py`; `src/apm_cli/utils/console.py` | | Compiled-output writes (atomic) | CompiledOutputWriter | `src/apm_cli/compilation/output_writer.py` | +| Generated-content footer ownership wording | compilation/footer.py (build_generation_footer) | `src/apm_cli/compilation/footer.py` | | Deployment provenance / state | deployment_ledger.py | `src/apm_cli/core/deployment_ledger.py` | | Target-scoped deployed-file contraction | install/manifest_reconcile.py (reconcile_target_deployed_files) | `src/apm_cli/install/manifest_reconcile.py` | | Install success / failure outcome | the canonical install-outcome path | `src/apm_cli/install/outcome.py` | diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md index fb8ad35e2..8c0174768 100644 --- a/.github/instructions/architecture.instructions.md +++ b/.github/instructions/architecture.instructions.md @@ -46,6 +46,7 @@ semicolon-delimited, and specific to the file(s) that own the fact. | Runtime descriptors | runtime/registry.py | `src/apm_cli/runtime/registry.py` | | User-facing output / diagnostics | CommandLogger / console owner | `src/apm_cli/core/command_logger.py`; `src/apm_cli/utils/console.py` | | Compiled-output writes (atomic) | CompiledOutputWriter | `src/apm_cli/compilation/output_writer.py` | +| Generated-content footer ownership wording | compilation/footer.py (build_generation_footer) | `src/apm_cli/compilation/footer.py` | | Deployment provenance / state | deployment_ledger.py | `src/apm_cli/core/deployment_ledger.py` | | Target-scoped deployed-file contraction | install/manifest_reconcile.py (reconcile_target_deployed_files) | `src/apm_cli/install/manifest_reconcile.py` | | Install success / failure outcome | the canonical install-outcome path | `src/apm_cli/install/outcome.py` | diff --git a/apm.lock.yaml b/apm.lock.yaml index e66145761..76bf6b48a 100644 --- a/apm.lock.yaml +++ b/apm.lock.yaml @@ -2783,7 +2783,7 @@ deployments: owners: - . active_owner: . - content_hash: sha256:3445ddcf51a14f5a730cddb7f3cc5ce2bb11b5078951521a4903a098250f6f86 + content_hash: sha256:7cefd135891c5022abfa5c509769a2debccb5c1a4d944153580a7a3d077beb37 - kind: project-relative target: copilot value: .github/instructions/changelog.instructions.md @@ -3290,7 +3290,7 @@ local_deployed_file_hashes: .github/agents/spec-tag-architect.agent.md: sha256:82907265c5e7cf1ac61ad96866fa7c5683b69c8f09b7a4c5f3cc241acc9568ca .github/agents/supply-chain-security-expert.agent.md: sha256:8fb8cc426d6af17ba084a28b3f026c2b475b62e3ca63ed2f88b83bd823f877af .github/agents/test-coverage-expert.agent.md: sha256:48c2172d1f18a394fa83ef9dc2be0b9b921a4e51e976498165250fed66369711 - .github/instructions/architecture.instructions.md: sha256:3445ddcf51a14f5a730cddb7f3cc5ce2bb11b5078951521a4903a098250f6f86 + .github/instructions/architecture.instructions.md: sha256:7cefd135891c5022abfa5c509769a2debccb5c1a4d944153580a7a3d077beb37 .github/instructions/changelog.instructions.md: sha256:1e51ec4c74e847967962bd279dc4c6e582c5d3578490b3c28d5f3acd3e05f73e .github/instructions/cicd.instructions.md: sha256:33201cb88ea2f34b4950a9b52f87dc8dfb682796aaf53068ba7ae406c0c5e2c2 .github/instructions/cli.instructions.md: sha256:8e39e8d5047ce88575cb02f87c2bcede584dfef258bd86f7466c7badf136541a diff --git a/docs/src/content/docs/producer/compile.md b/docs/src/content/docs/producer/compile.md index b53df8c78..7f6616f0d 100644 --- a/docs/src/content/docs/producer/compile.md +++ b/docs/src/content/docs/producer/compile.md @@ -245,7 +245,8 @@ you can omit `start_marker` and `end_marker` if you use those verbatim. - `start_marker` and `end_marker` must be distinct non-empty strings. - Content outside the markers is preserved verbatim across every compile run for the root `AGENTS.md`; only the block between the markers is - replaced. + replaced. When source attribution emits a footer, it identifies this block + as a generated section rather than describing the whole file as generated. - In distributed compile mode, subdirectory `AGENTS.md` files remain fully APM-owned and are overwritten on each run. diff --git a/scripts/check_agents_footer_authority.py b/scripts/check_agents_footer_authority.py new file mode 100644 index 000000000..5a8032a80 --- /dev/null +++ b/scripts/check_agents_footer_authority.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +"""Enforce one canonical owner for generated-content footer wording.""" + +from __future__ import annotations + +import ast +import sys +from pathlib import Path + +_OWNER = Path("src/apm_cli/compilation/footer.py") +_CONSUMERS = ( + Path("src/apm_cli/compilation/agents_compiler.py"), + Path("src/apm_cli/compilation/claude_formatter.py"), + Path("src/apm_cli/compilation/distributed_compiler.py"), + Path("src/apm_cli/compilation/template_builder.py"), +) +_OWNERSHIP_WORDING = "was generated by APM CLI. Do not edit manually." + + +def _string_constants(path: Path) -> list[str]: + """Return string constants from a Python source file.""" + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + return [ + node.value + for node in ast.walk(tree) + if isinstance(node, ast.Constant) and isinstance(node.value, str) + ] + + +def _calls_function(path: Path, function_name: str) -> bool: + """Return whether executable code calls the named function.""" + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + return any( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == function_name + for node in ast.walk(tree) + ) + + +def find_violations(root: Path) -> list[str]: + """Find duplicate footer wording or consumers bypassing the owner.""" + owner = root / _OWNER + owner_tree = ast.parse(owner.read_text(encoding="utf-8"), filename=str(owner)) + owner_definitions = [ + node + for node in owner_tree.body + if isinstance(node, ast.FunctionDef) and node.name == "build_generation_footer" + ] + violations: list[str] = [] + if len(owner_definitions) != 1: + violations.append(f"{_OWNER}: build_generation_footer must have exactly one definition") + + for relative_path in _CONSUMERS: + consumer = root / relative_path + if not _calls_function(consumer, "build_generation_footer"): + violations.append(f"{relative_path}: generated footer must use build_generation_footer") + + compilation_root = root / "src/apm_cli/compilation" + for source_path in compilation_root.rglob("*.py"): + if source_path == owner: + continue + if any(_OWNERSHIP_WORDING in value for value in _string_constants(source_path)): + relative_path = source_path.relative_to(root) + violations.append(f"{relative_path}: footer ownership wording duplicates {_OWNER}") + + return violations + + +def main() -> int: + """Run the generated-footer authority check.""" + root = Path(sys.argv[1]).resolve() if len(sys.argv) > 1 else Path(__file__).parents[1] + violations = find_violations(root) + if violations: + print("[x] Generated footer wording must route through compilation/footer.py") + for violation in violations: + print(violation) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/lint-architecture-boundaries.sh b/scripts/lint-architecture-boundaries.sh index fedf01cba..2d85084da 100755 --- a/scripts/lint-architecture-boundaries.sh +++ b/scripts/lint-architecture-boundaries.sh @@ -215,6 +215,12 @@ if [ "$agents_source_attribution_status" -ne 0 ]; then echo "$agents_source_attribution_output" violations=$((violations + 1)) fi +agents_footer_output=$(python3 scripts/check_agents_footer_authority.py "$ROOT" 2>&1) +agents_footer_status=$? +if [ "$agents_footer_status" -ne 0 ]; then + echo "$agents_footer_output" + violations=$((violations + 1)) +fi hook_file="src/apm_cli/integration/hook_integrator.py" validation_line=$(grep -n 'if not validation\.valid:' "$hook_file" | tail -1 | cut -d: -f1) continue_line=$(awk -v start="$validation_line" 'NR > start && /continue/ {print NR; exit}' "$hook_file") diff --git a/src/apm_cli/compilation/agents_compiler.py b/src/apm_cli/compilation/agents_compiler.py index b1a384d26..44d340512 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -27,6 +27,7 @@ from ..version import get_version from .claude_formatter import CLAUDE_HEADER, ClaudeFormatter from .constants import BUILD_ID_PLACEHOLDER +from .footer import VALID_AGENTS_MD_MODES, build_generation_footer from .inventory import CompileInventory from .link_resolver import resolve_markdown_links, validate_link_targets from .template_builder import ( @@ -178,11 +179,10 @@ def __post_init__(self): # Initialize exclude list if None if self.exclude is None: self.exclude = [] - _valid_modes = ("full", "managed_section") - if self.agents_md_mode not in _valid_modes: + if self.agents_md_mode not in VALID_AGENTS_MD_MODES: raise ValueError( f"Unknown agents_md.mode {self.agents_md_mode!r}. " - f"Supported values: {', '.join(repr(m) for m in _valid_modes)}." + f"Supported values: {', '.join(repr(mode) for mode in VALID_AGENTS_MD_MODES)}." ) @classmethod @@ -625,6 +625,7 @@ def _compile_distributed( "dry_run": config.dry_run, "skip_instructions": skip_instructions, "with_constitution": config.with_constitution, + "agents_md_mode": config.agents_md_mode, "placement_map": self._get_distributed_placement( config, primitives, @@ -1333,7 +1334,7 @@ def generate_output(self, template_data: TemplateData, config: CompilationConfig Returns: str: Generated AGENTS.md content. """ - content = generate_agents_md_template(template_data) + content = generate_agents_md_template(template_data, config.agents_md_mode) # Resolve markdown links if enabled if config.resolve_links: @@ -1524,10 +1525,7 @@ def _generate_copilot_root_instructions_content( sections.append("") if config.source_attribution: - sections.append("---") - sections.append("*This file was generated by APM CLI. Do not edit manually.*") - sections.append("*To regenerate: `apm compile`*") - sections.append("") + sections.extend(build_generation_footer()) content = "\n".join(sections) if config.resolve_links: @@ -1634,10 +1632,10 @@ def _prepare_output_content_with_config( ) except ManagedSectionError as exc: raise ManagedSectionError(f"[{target}] {exc}") from exc - elif config.agents_md_mode != "full": + elif config.agents_md_mode not in VALID_AGENTS_MD_MODES: raise ValueError( f"Unknown agents_md.mode {config.agents_md_mode!r}. " - "Supported values: 'full', 'managed_section'." + f"Supported values: {', '.join(repr(mode) for mode in VALID_AGENTS_MD_MODES)}." ) return content diff --git a/src/apm_cli/compilation/claude_formatter.py b/src/apm_cli/compilation/claude_formatter.py index 186bc9fc2..8f8fc5918 100644 --- a/src/apm_cli/compilation/claude_formatter.py +++ b/src/apm_cli/compilation/claude_formatter.py @@ -14,6 +14,7 @@ from ..version import get_version from .constants import BUILD_ID_PLACEHOLDER from .constitution import read_constitution +from .footer import build_generation_footer from .template_builder import build_attributed_instructions # CRITICAL: Shadow Click commands to prevent namespace collision @@ -337,10 +338,7 @@ def _generate_claude_content( # Footer is opt-in (cosmetic). if source_attribution: - sections.append("---") - sections.append("*This file was generated by APM CLI. Do not edit manually.*") - sections.append("*To regenerate: `apm compile`*") - sections.append("") + sections.extend(build_generation_footer()) return "\n".join(sections) diff --git a/src/apm_cli/compilation/distributed_compiler.py b/src/apm_cli/compilation/distributed_compiler.py index 96f39992c..d274dde02 100644 --- a/src/apm_cli/compilation/distributed_compiler.py +++ b/src/apm_cli/compilation/distributed_compiler.py @@ -20,6 +20,7 @@ from .constants import BUILD_ID_PLACEHOLDER from .constitution import find_constitution from .context_optimizer import ContextOptimizer +from .footer import build_generation_footer from .inventory import CompileInventory from .link_resolver import UnifiedLinkResolver from .template_builder import ( @@ -226,6 +227,7 @@ def compile_distributed( # Mirrors CompilationConfig.with_constitution: when False, the writer # skips constitution injection, so the emptiness predicate must agree. with_constitution = config.get("with_constitution", True) + agents_md_mode = config.get("agents_md_mode", "full") # Phase 0: Context Link Resolution # Register all context files and compile referenced ones @@ -315,6 +317,7 @@ def compile_distributed( primitives, skip_instructions=skip_instructions, source_attribution=source_attribution, + agents_md_mode=agents_md_mode, ) # Phase 4: Handle orphaned file cleanup. @@ -681,6 +684,7 @@ def _generate_agents_content( *, skip_instructions: bool = False, source_attribution: bool = True, + agents_md_mode: str = "full", ) -> str: """Generate AGENTS.md content for a specific placement. @@ -694,6 +698,8 @@ def _generate_agents_content( (APM version comment, generated-by footer), mirroring the CLAUDE.md path. Distinct from ``placement.source_attribution``, which is the per-instruction source map. + agents_md_mode: Root AGENTS.md ownership mode. Subdirectory files + always use full-file ownership wording. Returns: str: Generated AGENTS.md content. @@ -731,10 +737,9 @@ def _generate_agents_content( # Footer is opt-in (cosmetic). if source_attribution: - sections.append("---") - sections.append("*This file was generated by APM CLI. Do not edit manually.*") - sections.append("*To regenerate: `apm compile`*") - sections.append("") + is_root = placement.agents_path.parent == self.base_dir + footer_mode = agents_md_mode if is_root else "full" + sections.extend(build_generation_footer(footer_mode)) content = "\n".join(sections) diff --git a/src/apm_cli/compilation/footer.py b/src/apm_cli/compilation/footer.py new file mode 100644 index 000000000..d7292f57d --- /dev/null +++ b/src/apm_cli/compilation/footer.py @@ -0,0 +1,20 @@ +"""Canonical generated-content footer rendering.""" + +VALID_AGENTS_MD_MODES = ("full", "managed_section") + + +def build_generation_footer(agents_md_mode: str = "full") -> list[str]: + """Build a footer whose ownership wording matches the output mode.""" + if agents_md_mode not in VALID_AGENTS_MD_MODES: + raise ValueError( + f"Unknown agents_md.mode {agents_md_mode!r}. " + f"Supported values: {', '.join(repr(mode) for mode in VALID_AGENTS_MD_MODES)}." + ) + + subject = "section" if agents_md_mode == "managed_section" else "file" + return [ + "---", + f"*This {subject} was generated by APM CLI. Do not edit manually.*", + "*To regenerate: `apm compile`*", + "", + ] diff --git a/src/apm_cli/compilation/template_builder.py b/src/apm_cli/compilation/template_builder.py index ce09d77d1..d302263e3 100644 --- a/src/apm_cli/compilation/template_builder.py +++ b/src/apm_cli/compilation/template_builder.py @@ -6,6 +6,7 @@ from ..primitives.models import Chatmode, Instruction from ..utils.paths import portable_relpath +from .footer import build_generation_footer GLOBAL_INSTRUCTIONS_HEADING = "## Global Instructions" @@ -197,11 +198,12 @@ def find_chatmode_by_name(chatmodes: list[Chatmode], chatmode_name: str) -> Chat return None -def generate_agents_md_template(template_data: TemplateData) -> str: +def generate_agents_md_template(template_data: TemplateData, agents_md_mode: str = "full") -> str: """Generate the complete AGENTS.md file content. Args: template_data (TemplateData): Data for template generation. + agents_md_mode: Root AGENTS.md ownership mode. Returns: str: Complete AGENTS.md file content. @@ -226,10 +228,6 @@ def generate_agents_md_template(template_data: TemplateData) -> str: if template_data.instructions_content: sections.append(template_data.instructions_content) - # Footer - sections.append("---") - sections.append("*This file was generated by APM CLI. Do not edit manually.*") - sections.append("*To regenerate: `apm compile`*") - sections.append("") + sections.extend(build_generation_footer(agents_md_mode)) return "\n".join(sections) diff --git a/tests/integration/test_architecture_agents_footer.py b/tests/integration/test_architecture_agents_footer.py new file mode 100644 index 000000000..3fddf30f2 --- /dev/null +++ b/tests/integration/test_architecture_agents_footer.py @@ -0,0 +1,102 @@ +"""Architecture guardrails for generated-content footer wording.""" + +from __future__ import annotations + +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.component + + +def test_generated_footer_has_single_owner() -> None: + """All compilation renderers must use the canonical footer builder.""" + root = Path(__file__).parents[2] + owner = (root / "src/apm_cli/compilation/footer.py").read_text(encoding="utf-8") + guard = (root / "scripts/lint-architecture-boundaries.sh").read_text(encoding="utf-8") + architecture = (root / ".github/instructions/architecture.instructions.md").read_text( + encoding="utf-8" + ) + + assert owner.count("def build_generation_footer(") == 1 + assert "check_agents_footer_authority.py" in guard + assert "| Generated-content footer ownership wording |" in architecture + + +def test_generated_footer_guard_rejects_parallel_wording(tmp_path: Path) -> None: + """The boundary guard must reject footer wording duplicated by a renderer.""" + root = Path(__file__).parents[2] + sandbox = tmp_path / "repo" + paths = ( + "scripts/check_agents_footer_authority.py", + "src/apm_cli/compilation/agents_compiler.py", + "src/apm_cli/compilation/claude_formatter.py", + "src/apm_cli/compilation/distributed_compiler.py", + "src/apm_cli/compilation/footer.py", + "src/apm_cli/compilation/template_builder.py", + ) + for relative_path in paths: + source = root / relative_path + destination = sandbox / relative_path + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(source, destination) + + consumer = sandbox / "src/apm_cli/compilation/template_builder.py" + consumer.write_text( + consumer.read_text(encoding="utf-8") + + '\n_DUPLICATE_FOOTER = "This file was generated by APM CLI. ' + + 'Do not edit manually."\n', + encoding="utf-8", + ) + + result = subprocess.run( + (sys.executable, "scripts/check_agents_footer_authority.py", str(sandbox)), + cwd=sandbox, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "Generated footer wording must route through compilation/footer.py" in result.stdout + + +def test_generated_footer_guard_requires_executable_consumer_call(tmp_path: Path) -> None: + """A comment mentioning the owner must not satisfy the boundary guard.""" + root = Path(__file__).parents[2] + sandbox = tmp_path / "repo" + paths = ( + "scripts/check_agents_footer_authority.py", + "src/apm_cli/compilation/agents_compiler.py", + "src/apm_cli/compilation/claude_formatter.py", + "src/apm_cli/compilation/distributed_compiler.py", + "src/apm_cli/compilation/footer.py", + "src/apm_cli/compilation/template_builder.py", + ) + for relative_path in paths: + source = root / relative_path + destination = sandbox / relative_path + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(source, destination) + + consumer = sandbox / "src/apm_cli/compilation/template_builder.py" + source = consumer.read_text(encoding="utf-8") + source = source.replace( + "sections.extend(build_generation_footer(agents_md_mode))", + "sections.extend([]) # build_generation_footer( is not executable", + ) + consumer.write_text(source, encoding="utf-8") + + result = subprocess.run( + (sys.executable, "scripts/check_agents_footer_authority.py", str(sandbox)), + cwd=sandbox, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "template_builder.py: generated footer must use" in result.stdout diff --git a/tests/unit/compilation/test_agents_footer_2689.py b/tests/unit/compilation/test_agents_footer_2689.py new file mode 100644 index 000000000..a432a8936 --- /dev/null +++ b/tests/unit/compilation/test_agents_footer_2689.py @@ -0,0 +1,105 @@ +"""Regression coverage for mode-aware generated footers (issue #2689).""" + +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from apm_cli.cli import cli +from apm_cli.compilation.distributed_compiler import ( + DistributedAgentsCompiler, + PlacementResult, +) +from apm_cli.compilation.footer import build_generation_footer +from apm_cli.primitives.models import PrimitiveCollection + +pytestmark = pytest.mark.component + +_FILE_FOOTER = "*This file was generated by APM CLI. Do not edit manually.*" +_SECTION_FOOTER = "*This section was generated by APM CLI. Do not edit manually.*" +_START_MARKER = "" +_END_MARKER = "" + + +def test_full_mode_footer_retains_whole_file_wording() -> None: + """The default mode must preserve the existing whole-file contract.""" + footer = build_generation_footer() + + assert _FILE_FOOTER in footer + assert _SECTION_FOOTER not in footer + + +def _create_managed_project(project: Path) -> Path: + """Create a minimal project with a managed root AGENTS.md.""" + agents_md = project / "AGENTS.md" + agents_md.write_text( + "# Team guidance\n\n" + "Human-authored content.\n\n" + f"{_START_MARKER}\n" + "Old APM block.\n" + f"{_END_MARKER}\n\n" + "Human footer.\n", + encoding="utf-8", + ) + (project / "apm.yml").write_text( + "name: test-project\n" + "version: 0.1.0\n" + "compilation:\n" + " source_attribution: true\n" + " agents_md:\n" + " mode: managed_section\n", + encoding="utf-8", + ) + instructions_dir = project / ".apm" / "instructions" + instructions_dir.mkdir(parents=True) + (instructions_dir / "coding.instructions.md").write_text( + "---\n" + "description: Test instructions\n" + 'applyTo: "**/*.py"\n' + "---\n\n" + "# Test instructions\n\n" + "Use the project style.\n", + encoding="utf-8", + ) + return agents_md + + +@pytest.mark.parametrize( + "compile_args", + [ + pytest.param([], id="distributed"), + pytest.param(["--single-agents"], id="single-file"), + ], +) +def test_generated_footer_describes_managed_section( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, compile_args: list[str] +) -> None: + """Every root AGENTS.md strategy must describe section ownership.""" + agents_md = _create_managed_project(tmp_path) + monkeypatch.chdir(tmp_path) + + result = CliRunner().invoke(cli, ["compile", *compile_args, "--local-only"]) + + assert result.exit_code == 0, result.output + written = agents_md.read_text(encoding="utf-8") + start = written.index(_START_MARKER) + end = written.index(_END_MARKER) + managed_content = written[start:end] + assert _SECTION_FOOTER in managed_content + assert _FILE_FOOTER not in managed_content + + +def test_managed_mode_keeps_subdirectory_footer_file_scoped(tmp_path: Path) -> None: + """A fully generated nested AGENTS.md must retain file ownership wording.""" + compiler = DistributedAgentsCompiler(base_dir=str(tmp_path)) + placement = PlacementResult( + agents_path=tmp_path / "services" / "api" / "AGENTS.md", + instructions=[], + ) + + nested_content = compiler._generate_agents_content( + placement, PrimitiveCollection(), agents_md_mode="managed_section" + ) + + assert _FILE_FOOTER in nested_content + assert _SECTION_FOOTER not in nested_content