diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c6f48c56..7693837b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`apm compile -t copilot` now emits `.github/copilot-instructions.md` with zero user configuration** -- APM's first Copilot-native compile target. Global instructions in `.apm/instructions/` are assembled into the file VS Code and GitHub Copilot read automatically; switching targets cleans it up. APM dogfoods this target. (#1048) - **`apm marketplace add` accepts full HTTPS URLs and nested HOST/group/sub/.../REPO shorthands.** You can now paste a repository URL straight from the browser (e.g., `apm marketplace add https://github.com/acme/plugin-marketplace`) and register marketplaces hosted under nested sub-paths on GitHub Enterprise (`ghes.corp.example.com/org/team/repo`). Path-traversal sequences in the parsed segments are rejected via `validate_path_segments`. Non-GitHub hosts (GitLab, Bitbucket, etc.) are explicitly rejected at registration time with an actionable error -- this avoids forwarding GitHub credentials to unintended hosts and the silent fetch-time 404 that previously resulted; native non-GitHub support is tracked separately. (#1034, closes #1027) - Regression tests for `apm compile` placement of narrow `applyTo` patterns: instructions whose matches all live deep inside one subtree are now pinned to the deepest covering directory instead of being hoisted to the project root, across both selective and single-point placement strategies. Also covers the file-walk cache that skips repeated filesystem scans for the same glob. (#871) - **`apm pack` marketplace builder hardening.** Local source paths are now emitted relative to `metadata.pluginRoot` (fixes double-prefix bug). New pass-through fields: `author`, `license`, `repository`, `keywords` (alias for `tags`). Curator-wins override semantics for `description`/`version` on remote entries. Security guards reject path traversal and absolute paths post-subtraction. (#1061) @@ -24,6 +25,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `apm compile --dry-run -t copilot` now faithfully simulates the hand-authored file guard: a `.github/copilot-instructions.md` lacking the APM marker is reported as `skipped=1` (matching the real run) instead of as `generated=1`. Previously dry-run would claim a write that a real run would refuse, giving CI preview gates a false signal. (#1048) +- `apm compile -t claude,copilot` (and any multi-target list including `copilot`) now correctly generates `.github/copilot-instructions.md`; previously it was silently skipped on the multi-target code path. (#1048) +- `apm compile` no longer overwrites a hand-authored `.github/copilot-instructions.md`; if the file lacks the APM-generated marker, regeneration is skipped with a warning that names the literal marker line (``) so users can self-serve recovery. **Migration:** to adopt APM management of an existing file, either delete or rename it and re-run `apm compile`, or prepend the marker line to the top of the file and re-run `apm compile`. (#1048) +- Generated footer in `.github/copilot-instructions.md` now reads `apm compile` (was `specify apm compile`). (#1048) +- `apm compile --targets claude` no longer lists `@apm_modules/{owner}/{package}/CLAUDE.md` dependencies for packages that don't have a `CLAUDE.md` file on disk (#1047) - **`apm prune` no longer flags directories it put there itself** -- skills installed from a subdirectory path (e.g., `owner/repo/.apm/skills/skill-name`) no longer cause the parent `owner/repo/` clone to appear as an orphan. Fixes spurious removal prompts in multi-skill and monorepo-style setups. The same fix applies to `apm deps list` and `apm compile`. A genuinely orphaned `owner/repo` package is still flagged even when a sibling subdirectory dep shares the same `owner/repo` root. No changes to `apm.yml` or the lockfile are required to benefit from this fix. (#1050) ## [0.11.0] - 2026-04-29 diff --git a/README.md b/README.md index cb1f6d94c..d84b8bb82 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,14 @@ apm install vercel-labs/agent-skills --skill deploy-to-vercel # one skill, per Same install gesture. You also get a [manifest, lockfile, and reproducibility](https://microsoft.github.io/apm/reference/package-types/#skill-collection-skillsnameskillmd). +**Zero-config Copilot:** + +```bash +apm compile -t copilot # writes .github/copilot-instructions.md +``` + +One command, no configuration -- VS Code and GitHub Copilot read the file automatically. APM dogfoods this target on its own repository. + ## The three promises ### 1. Portable by manifest diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index 14698f4c6..9d649d918 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -121,7 +121,7 @@ download → scan source → block or deploy → report Content scanning extends beyond install: -- **`apm compile`** scans compiled output (AGENTS.md, CLAUDE.md, commands) before writing to disk. Critical findings cause `apm compile` to exit with code 1 after writing — defense-in-depth since source files were already scanned at install, but compilation assembles content from multiple sources. +- **`apm compile`** scans compiled output (AGENTS.md, CLAUDE.md, `.github/copilot-instructions.md`, commands) before writing to disk. Critical findings cause `apm compile` to exit with code 1 after writing — defense-in-depth since source files were already scanned at install, but compilation assembles content from multiple sources. `.github/copilot-instructions.md` is assembled from global instructions in `.apm/instructions/`, including those installed under `apm_modules/`. - **`apm pack`** scans files before bundling. This catches hidden characters before a package is published, preventing authors from accidentally distributing tainted content. - **`apm unpack`** scans bundle contents before deployment. This is a pre-deployment gate matching `apm install` — critical findings block deployment unless `--force` is used. diff --git a/docs/src/content/docs/getting-started/quick-start.md b/docs/src/content/docs/getting-started/quick-start.md index 7b772d040..ca0c780f1 100644 --- a/docs/src/content/docs/getting-started/quick-start.md +++ b/docs/src/content/docs/getting-started/quick-start.md @@ -113,6 +113,16 @@ dependencies: - microsoft/apm-sample-package#v1.0.0 ``` +## Get Copilot reading your packages in under a minute + +Run one more command: + +```bash +apm compile -t copilot +``` + +APM assembles every global instruction it just installed into `.github/copilot-instructions.md` -- the file VS Code and GitHub Copilot read automatically. No configuration, no extra setup; open the project in VS Code and Copilot is already grounded in your packages' standards. + ## That's it Open your editor. GitHub Copilot, Claude, Cursor, and OpenCode pick up the new context immediately -- no extra configuration, no compile step, no restart. The agent now knows your project's design standards, can run your prompt templates, and follows the conventions defined in the package. diff --git a/docs/src/content/docs/reference/cli-commands.md b/docs/src/content/docs/reference/cli-commands.md index b960b5cf3..f8033fe67 100644 --- a/docs/src/content/docs/reference/cli-commands.md +++ b/docs/src/content/docs/reference/cli-commands.md @@ -1748,13 +1748,13 @@ apm compile --watch apm compile --watch --dry-run # Target specific agent formats -apm compile --target vscode # AGENTS.md + .github/ only +apm compile --target vscode # AGENTS.md + .github/ (incl. copilot-instructions.md) apm compile --target claude # CLAUDE.md + .claude/ only apm compile --target opencode # AGENTS.md + .opencode/ only apm compile --target all # All formats (default) # Multiple targets (comma-separated) -apm compile -t claude,copilot # Both CLAUDE.md and AGENTS.md +apm compile -t claude,copilot # CLAUDE.md + AGENTS.md + .github/copilot-instructions.md # Compile injecting Spec Kit constitution (auto-detected) apm compile --with-constitution @@ -1779,6 +1779,9 @@ apm compile --no-constitution **Content Scanning:** Compiled output is scanned for hidden Unicode characters before writing to disk. Critical findings cause `apm compile` to exit with code 1 — defense-in-depth since source files are already scanned during `apm install`. +**`.github/copilot-instructions.md` generation:** +When the resolved target is `copilot` (alias `vscode`), `all`, or any multi-target list containing `copilot`, `apm compile` assembles all *global* instructions (entries in `.apm/instructions/` without an `apply_to` field) into `.github/copilot-instructions.md` -- the file VS Code and GitHub Copilot read automatically with zero user configuration. Generated content is wrapped with an APM-only marker (literal first line: ``). Switching to a non-Copilot target (e.g. `apm compile -t claude`) cleans up the file only when the marker is present; a hand-authored `.github/copilot-instructions.md` is left untouched on both write and cleanup paths. To adopt APM management of an existing hand-authored file, delete (or rename) it and re-run `apm compile`, or prepend the marker line `` to the top of the file and re-run `apm compile`. + **Configuration Integration:** The compile command supports configuration via `apm.yml`: diff --git a/src/apm_cli/commands/compile/cli.py b/src/apm_cli/commands/compile/cli.py index 3f8fb87c7..81b040417 100644 --- a/src/apm_cli/commands/compile/cli.py +++ b/src/apm_cli/commands/compile/cli.py @@ -185,25 +185,47 @@ def _resolve_compile_target(target): return None # will trigger detect_target() auto-detection if isinstance(target, list): target_set = set(target) - agents_family = {"copilot", "vscode", "agents", "cursor", "opencode", "codex"} - has_agents_family = bool(target_set & agents_family) + # Two distinct families overlap on copilot/vscode/agents: + # copilot_family -> requests .github/copilot-instructions.md AND AGENTS.md + # agents_md_family -> requests AGENTS.md only (cursor/opencode/codex) + # Splitting these prevents the over-fire bug where -t cursor,claude or + # -t cursor,opencode,codex used to incorrectly emit copilot-instructions.md. + copilot_family = {"copilot", "vscode", "agents"} + agents_md_family = {"cursor", "opencode", "codex"} + has_copilot = bool(target_set & copilot_family) + has_agents_md_only = bool(target_set & agents_md_family) has_claude = "claude" in target_set has_gemini = "gemini" in target_set families = set() - if has_agents_family: - families.add("agents") + if has_copilot: + families.add("vscode") # gates copilot-instructions.md + families.add("agents") # also gates AGENTS.md + elif has_agents_md_only: + families.add("agents") # AGENTS.md only -- no copilot-instructions if has_claude: families.add("claude") if has_gemini: families.add("gemini") if len(families) >= 2: + # Single-target copilot collapses {"vscode","agents"} to bare "vscode" + # for routing parity with single-string -t copilot. + if families == {"vscode", "agents"}: + return "vscode" return frozenset(families) elif has_claude: return "claude" elif has_gemini: return "gemini" + elif has_copilot: + return "vscode" else: - return "vscode" # agents-family only + # cursor/opencode/codex only -- preserve the bare target name so + # single-element list routing matches single-string semantics + # (-t cursor and -t cursor both end up as "cursor"). + for bare in ("cursor", "opencode", "codex"): + if bare in target_set: + return bare + return "vscode" # defensive fallback (unreachable) return target # single string pass-through diff --git a/src/apm_cli/compilation/agents_compiler.py b/src/apm_cli/compilation/agents_compiler.py index 44819130d..e8ee41700 100644 --- a/src/apm_cli/compilation/agents_compiler.py +++ b/src/apm_cli/compilation/agents_compiler.py @@ -53,7 +53,7 @@ # Compiler families allowed inside a multi-target frozenset (built by # _resolve_compile_target() from CLI-validated target names). Kept narrow # because the frozenset path bypasses _KNOWN_TARGETS validation. -_KNOWN_COMPILE_FAMILIES = frozenset({"agents", "claude", "gemini"}) +_KNOWN_COMPILE_FAMILIES = frozenset({"agents", "vscode", "claude", "gemini"}) @dataclass @@ -883,7 +883,14 @@ def _maybe_emit_copilot_root_instructions( primitives: PrimitiveCollection, result: CompilationResult, ) -> CompilationResult: - """Generate .github/copilot-instructions.md for Copilot-capable targets.""" + """Generate .github/copilot-instructions.md for Copilot-capable targets. + + Skip semantics: if the file already exists without the APM-generated + marker, it is treated as hand-authored and left untouched. The + ``copilot_root_instructions_skipped`` stat captures this case + explicitly so callers can distinguish it from a genuine no-op + (``copilot_root_instructions_unchanged``) or an unrouted target. + """ routing_target = "vscode" if config.target in _VSCODE_TARGET_ALIASES else config.target output_path = self.base_dir / ".github" / "copilot-instructions.md" if not should_compile_copilot_instructions_md(routing_target): @@ -892,6 +899,7 @@ def _maybe_emit_copilot_root_instructions( result.stats.setdefault("copilot_root_instructions_generated", 0) result.stats.setdefault("copilot_root_instructions_written", 0) result.stats.setdefault("copilot_root_instructions_unchanged", 0) + result.stats.setdefault("copilot_root_instructions_skipped", 0) result.stats.setdefault("copilot_root_instructions_removed", 0) return result @@ -905,17 +913,54 @@ def _maybe_emit_copilot_root_instructions( result.stats.setdefault("copilot_root_instructions_generated", 0) result.stats.setdefault("copilot_root_instructions_written", 0) result.stats.setdefault("copilot_root_instructions_unchanged", 0) + result.stats.setdefault("copilot_root_instructions_skipped", 0) result.stats.setdefault("copilot_root_instructions_removed", 0) return result content = self._generate_copilot_root_instructions_content(global_instructions, config) result.stats["copilot_root_instructions_generated"] = 1 + result.stats.setdefault("copilot_root_instructions_skipped", 0) result.stats.setdefault("copilot_root_instructions_removed", 0) + result.stats.setdefault("copilot_root_instructions_written", 0) + result.stats.setdefault("copilot_root_instructions_unchanged", 0) + + # Inspect any existing file BEFORE the dry-run early-exit so that + # `--dry-run` faithfully reports what a real run would do (skip vs + # write vs unchanged). Reading the file here is safe in dry-run mode + # because we never mutate it. + try: + existing = output_path.read_text(encoding="utf-8") if output_path.exists() else None + except OSError as exc: + message = f"Failed to read {output_path}: {exc}" + self.errors.append(message) + result.errors.append(message) + result.success = False + return result + + if existing is not None and _COPILOT_ROOT_GENERATED_MARKER not in existing: + rel_path = portable_relpath(output_path, self.base_dir) + result.warnings.append( + f"Skipped {rel_path}: hand-authored file will not be overwritten. " + "To regenerate, either delete or rename it, or prepend the line " + f"'{_COPILOT_ROOT_GENERATED_MARKER}' to the top of the file. " + "Then re-run 'apm compile'." + ) + # The file was never compared to new content; record as + # 'skipped', not 'unchanged'. Also reset 'generated' since no + # output was actually emitted (or would be, on a real run). + result.stats["copilot_root_instructions_generated"] = 0 + result.stats["copilot_root_instructions_written"] = 0 + result.stats["copilot_root_instructions_skipped"] = 1 + result.stats["copilot_root_instructions_unchanged"] = 0 + return result + + if existing == content: + result.stats["copilot_root_instructions_written"] = 0 + result.stats["copilot_root_instructions_unchanged"] = 1 + return result if config.dry_run: - result.stats.setdefault("copilot_root_instructions_written", 0) - result.stats.setdefault("copilot_root_instructions_unchanged", 0) return result from ..security.gate import WARN_POLICY, SecurityGate @@ -932,12 +977,6 @@ def _maybe_emit_copilot_root_instructions( try: output_path.parent.mkdir(parents=True, exist_ok=True) - existing = output_path.read_text(encoding="utf-8") if output_path.exists() else None - if existing == content: - result.stats["copilot_root_instructions_written"] = 0 - result.stats["copilot_root_instructions_unchanged"] = 1 - return result - output_path.write_text(content, encoding="utf-8") result.stats["copilot_root_instructions_written"] = 1 result.stats["copilot_root_instructions_unchanged"] = 0 @@ -973,7 +1012,7 @@ def _generate_copilot_root_instructions_content( sections.append("---") sections.append("*This file was generated by APM CLI. Do not edit manually.*") - sections.append("*To regenerate: `specify apm compile`*") + sections.append("*To regenerate: `apm compile`*") sections.append("") content = "\n".join(sections) diff --git a/src/apm_cli/compilation/claude_formatter.py b/src/apm_cli/compilation/claude_formatter.py index 945f8d215..2641cdbb8 100644 --- a/src/apm_cli/compilation/claude_formatter.py +++ b/src/apm_cli/compilation/claude_formatter.py @@ -212,7 +212,7 @@ def _collect_dependencies(self) -> builtins.list[str]: dependencies = [] apm_modules_dir = self.base_dir / "apm_modules" - if not apm_modules_dir.exists(): + if not apm_modules_dir.is_dir(): return dependencies # Scan for CLAUDE.md files in apm_modules @@ -225,6 +225,10 @@ def _collect_dependencies(self) -> builtins.list[str]: if not package_dir.is_dir() or package_dir.name.startswith("."): continue + claude_md_path = package_dir / "CLAUDE.md" + if not claude_md_path.is_file(): + continue + # Build the @import path import_path = f"@apm_modules/{owner_dir.name}/{package_dir.name}/CLAUDE.md" dependencies.append(import_path) diff --git a/src/apm_cli/core/target_detection.py b/src/apm_cli/core/target_detection.py index 9474e281c..6f2c977d5 100644 --- a/src/apm_cli/core/target_detection.py +++ b/src/apm_cli/core/target_detection.py @@ -32,7 +32,16 @@ # Compiler families used inside a multi-target frozenset. Narrower than # TargetType because the families are produced by _resolve_compile_target() # (in the compile CLI) from CLI-validated target names. -CompileFamily = Literal["agents", "claude", "gemini"] +# +# Family semantics: +# "agents" -> AGENTS.md is generated (any of copilot/vscode/agents/cursor/ +# opencode/codex was requested) +# "vscode" -> .github/copilot-instructions.md is generated (only when +# copilot/vscode/agents was specifically requested -- NOT for +# cursor/opencode/codex which use their own native config files) +# "claude" -> CLAUDE.md is generated +# "gemini" -> GEMINI.md is generated +CompileFamily = Literal["agents", "vscode", "claude", "gemini"] # Compile target: either a single TargetType string or a frozenset of compiler # families ({"agents", "claude", "gemini"}) for multi-target lists. @@ -196,15 +205,27 @@ def should_compile_gemini_md(target: CompileTargetType) -> bool: return target in ("gemini", "all") -def should_compile_copilot_instructions_md(target: TargetType) -> bool: +def should_compile_copilot_instructions_md(target: CompileTargetType) -> bool: """Check if .github/copilot-instructions.md should be compiled. + Only the Copilot-native targets (copilot/vscode/agents alias) and "all" + trigger generation. cursor, opencode, and codex use their own native + configuration files and must NOT receive copilot-instructions.md, even + when combined in a multi-target list. + Args: - target: The detected or configured target + target: The detected or configured target. May be a string or a + frozenset of compiler families for multi-target lists. Returns: bool: True if Copilot root instructions should be generated """ + if isinstance(target, frozenset): + # "vscode" family is added to the frozenset by _resolve_compile_target() + # ONLY when copilot/vscode/agents was in the original list. Checking + # "agents" would over-fire because cursor/opencode/codex also map to + # the "agents" family for AGENTS.md generation. + return "vscode" in target return target in ("vscode", "all") diff --git a/tests/unit/compilation/test_claude_formatter.py b/tests/unit/compilation/test_claude_formatter.py index c8265cfc7..af1363959 100644 --- a/tests/unit/compilation/test_claude_formatter.py +++ b/tests/unit/compilation/test_claude_formatter.py @@ -201,14 +201,27 @@ class TestDependenciesImportSyntax: @pytest.fixture def temp_project_with_deps(self): - """Create a temporary project with apm_modules dependencies.""" + """Create a temporary project with apm_modules containing packages with and without CLAUDE.md.""" temp_dir = tempfile.mkdtemp() temp_path = Path(temp_dir).resolve() # Create apm_modules structure apm_modules = temp_path / "apm_modules" + # Packages WITH CLAUDE.md - should be included (apm_modules / "owner1" / "package1").mkdir(parents=True) + (apm_modules / "owner1" / "package1" / "CLAUDE.md").write_text( + "# Package 1", encoding="utf-8" + ) (apm_modules / "owner2" / "package2").mkdir(parents=True) + (apm_modules / "owner2" / "package2" / "CLAUDE.md").write_text( + "# Package 2", encoding="utf-8" + ) + # Packages WITHOUT CLAUDE.md - should be excluded + (apm_modules / "owner1" / "no-claude").mkdir(parents=True) + (apm_modules / "owner1" / "no-claude" / "README.md").write_text( + "# No Claude", encoding="utf-8" + ) + (apm_modules / "owner3" / "skills-only").mkdir(parents=True) yield temp_path shutil.rmtree(temp_dir, ignore_errors=True) @@ -254,6 +267,14 @@ def test_no_dependencies_section_when_no_apm_modules(self, tmp_path): assert deps == [] + def test_skips_packages_without_claude_md(self, temp_project_with_deps): + """Test that packages without a CLAUDE.md file are not listed as dependencies.""" + formatter = ClaudeFormatter(str(temp_project_with_deps)) + deps = formatter._collect_dependencies() + + assert "@apm_modules/owner1/no-claude/CLAUDE.md" not in deps + assert "@apm_modules/owner3/skills-only/CLAUDE.md" not in deps + class TestAgentsExcludedFromClaudeMd: """Tests verifying agents/workflows are NOT included in CLAUDE.md. diff --git a/tests/unit/compilation/test_compile_target_flag.py b/tests/unit/compilation/test_compile_target_flag.py index 9a738e49b..c3525d2d1 100644 --- a/tests/unit/compilation/test_compile_target_flag.py +++ b/tests/unit/compilation/test_compile_target_flag.py @@ -380,6 +380,313 @@ def test_cleanup_preserves_unmanaged_manual_copilot_root_instructions(self, temp assert root_file.read_text(encoding="utf-8") == "Manual instructions\n" assert result.stats["copilot_root_instructions_removed"] == 0 + def test_write_path_preserves_unmanaged_manual_copilot_root_instructions(self, temp_project): + """Write path must not overwrite a hand-authored copilot-instructions.md. + + Regression for the missing marker guard on the write path: a manually + authored .github/copilot-instructions.md must survive the first + `apm compile -t copilot` even when global instructions exist. + """ + root_file = temp_project / ".github" / "copilot-instructions.md" + root_file.parent.mkdir(parents=True) + manual_content = "# Manual\n\nHand-authored guidance.\n" + root_file.write_text(manual_content, encoding="utf-8") + + primitives = PrimitiveCollection() + primitives.add_primitive( + Instruction( + name="contributing", + file_path=temp_project / ".apm/instructions/contributing.instructions.md", + description="General contributing guidance", + apply_to="", + content="# Contributing\n\nRun focused tests first.", + author="test", + source="local", + ) + ) + + compiler = AgentsCompiler(str(temp_project)) + result = compiler.compile( + CompilationConfig(target="vscode", dry_run=False, single_agents=True), + primitives, + ) + + assert result.success + assert root_file.read_text(encoding="utf-8") == manual_content + assert result.stats["copilot_root_instructions_written"] == 0 + assert any("hand-authored file will not be overwritten" in w for w in result.warnings) + assert any( + "" in w for w in result.warnings + ), "warning must disclose the literal marker string for self-service recovery" + + def test_multi_target_claude_copilot_emits_copilot_root_instructions(self, temp_project): + """`apm compile -t claude,copilot` (frozenset path) must emit copilot-instructions.md. + + Regression for the multi-target frozenset gap: should_compile_copilot_instructions_md + used to ignore the frozenset branch and silently skip generation. + """ + primitives = PrimitiveCollection() + primitives.add_primitive( + Instruction( + name="contributing", + file_path=temp_project / ".apm/instructions/contributing.instructions.md", + description="General contributing guidance", + apply_to="", + content="# Contributing\n\nRun focused tests first.", + author="test", + source="local", + ) + ) + + compiler = AgentsCompiler(str(temp_project)) + result = compiler.compile( + CompilationConfig( + target=frozenset({"vscode", "agents", "claude"}), + dry_run=False, + single_agents=True, + ), + primitives, + ) + + assert result.success + root_file = temp_project / ".github" / "copilot-instructions.md" + assert root_file.exists(), ( + "copilot-instructions.md should be generated when 'vscode' is in the frozenset target" + ) + assert result.stats["copilot_root_instructions_generated"] == 1 + + def test_frozenset_cursor_opencode_codex_does_not_emit_copilot_instructions(self, temp_project): + """Round-3 regression: -t cursor,opencode,codex (no copilot family) + must NOT emit .github/copilot-instructions.md. + + Previously the 'agents' family token was overloaded for AGENTS.md AND + copilot-instructions.md routing, causing the multi-target path to + over-fire for AGENTS.md-only combos. + """ + primitives = PrimitiveCollection() + primitives.add_primitive( + Instruction( + name="contributing", + file_path=temp_project / ".apm/instructions/contributing.instructions.md", + description="General contributing guidance", + apply_to="", + content="# Contributing\n\nRun focused tests first.", + author="test", + source="local", + ) + ) + + compiler = AgentsCompiler(str(temp_project)) + # Simulate what _resolve_compile_target(["cursor","opencode","codex"]) returns + # under the new semantics: a single 'agents'-only family collapses to + # the bare cursor target, which routes correctly via the string path. + result = compiler.compile( + CompilationConfig(target="cursor", dry_run=False, single_agents=True), + primitives, + ) + + assert result.success + root_file = temp_project / ".github" / "copilot-instructions.md" + assert not root_file.exists(), ( + "copilot-instructions.md must NOT be generated for cursor/opencode/codex" + ) + assert result.stats.get("copilot_root_instructions_generated", 0) == 0 + + def test_resolve_compile_target_cursor_opencode_codex_does_not_route_to_vscode(self): + """Round-3 regression at the resolver: cursor/opencode/codex multi-target + list must not collapse to 'vscode' (which would wrongly route + copilot-instructions.md generation).""" + from apm_cli.commands.compile.cli import _resolve_compile_target + from apm_cli.core.target_detection import should_compile_copilot_instructions_md + + for target_list in ( + ["cursor"], + ["opencode"], + ["codex"], + ["cursor", "opencode"], + ["cursor", "opencode", "codex"], + ): + resolved = _resolve_compile_target(target_list) + assert resolved != "vscode", ( + f"_resolve_compile_target({target_list!r}) must not return 'vscode' " + f"-- got {resolved!r}" + ) + assert should_compile_copilot_instructions_md(resolved) is False, ( + f"copilot-instructions.md must not be routed for {target_list!r}" + ) + + def test_resolve_compile_target_copilot_family_combos_do_route(self): + """Cross-check: copilot-family multi-target combos still route to + copilot-instructions.md generation under the new predicate.""" + from apm_cli.commands.compile.cli import _resolve_compile_target + from apm_cli.core.target_detection import should_compile_copilot_instructions_md + + for target_list in ( + ["copilot"], + ["vscode"], + ["claude", "copilot"], + ["claude", "vscode"], + ["gemini", "vscode"], + ["cursor", "vscode"], + ["claude", "vscode", "gemini"], + ): + resolved = _resolve_compile_target(target_list) + assert should_compile_copilot_instructions_md(resolved) is True, ( + f"copilot-instructions.md must be routed for {target_list!r} " + f"-- resolver returned {resolved!r}" + ) + + def test_skipped_hand_authored_file_records_skipped_stat_not_unchanged(self, temp_project): + """Round-3 regression: the skip-on-hand-authored-file path must record + 'skipped' in stats and reset 'generated' to 0, not leave it at 1 + (false positive) or use 'unchanged' (semantically wrong).""" + root_file = temp_project / ".github" / "copilot-instructions.md" + root_file.parent.mkdir(parents=True) + root_file.write_text("# Hand-authored\n", encoding="utf-8") + + primitives = PrimitiveCollection() + primitives.add_primitive( + Instruction( + name="contributing", + file_path=temp_project / ".apm/instructions/contributing.instructions.md", + description="General contributing guidance", + apply_to="", + content="# Contributing\n\nRun focused tests first.", + author="test", + source="local", + ) + ) + + compiler = AgentsCompiler(str(temp_project)) + result = compiler.compile( + CompilationConfig(target="vscode", dry_run=False, single_agents=True), + primitives, + ) + + assert result.success + assert result.stats["copilot_root_instructions_generated"] == 0, ( + "generated must be 0 when the hand-authored guard fires (no file was emitted)" + ) + assert result.stats["copilot_root_instructions_written"] == 0 + assert result.stats["copilot_root_instructions_skipped"] == 1, ( + "skipped stat must be 1 to distinguish from a genuine no-op" + ) + assert result.stats["copilot_root_instructions_unchanged"] == 0, ( + "unchanged means content was compared and matched -- not the case here" + ) + + def test_skip_warning_uses_relative_path_and_concrete_action(self, temp_project): + """Round-3 UX fix: warning must use a relative path and tell the user + exactly what command to run after removing the file.""" + root_file = temp_project / ".github" / "copilot-instructions.md" + root_file.parent.mkdir(parents=True) + root_file.write_text("# Hand-authored\n", encoding="utf-8") + + primitives = PrimitiveCollection() + primitives.add_primitive( + Instruction( + name="contributing", + file_path=temp_project / ".apm/instructions/contributing.instructions.md", + description="General contributing guidance", + apply_to="", + content="# Contributing\n\nRun focused tests first.", + author="test", + source="local", + ) + ) + + compiler = AgentsCompiler(str(temp_project)) + result = compiler.compile( + CompilationConfig(target="vscode", dry_run=False, single_agents=True), + primitives, + ) + + skip_warnings = [ + w for w in result.warnings if "hand-authored file will not be overwritten" in w + ] + assert skip_warnings, "expected a skip warning to be emitted" + warning = skip_warnings[0] + assert ".github/copilot-instructions.md" in warning, "warning must reference the file" + assert str(temp_project) not in warning, ( + "warning must not embed the absolute project path (use a relative path)" + ) + assert "apm compile" in warning, "warning must tell the user the next command to run" + assert "delete or rename" in warning, "warning must give a concrete action" + assert "" in warning, ( + "warning must disclose the literal marker string so users can self-serve recovery" + ) + + def test_dry_run_respects_hand_authored_guard(self, temp_project): + """Round-4 regression: `--dry-run` must mirror the hand-authored guard + the real run applies, not silently report `generated=1` for a file the + real run would skip. CI scripts that use `--dry-run` as a preview gate + depend on the simulation being faithful.""" + root_file = temp_project / ".github" / "copilot-instructions.md" + root_file.parent.mkdir(parents=True) + manual_content = "# Hand-authored\n\nNo APM marker present.\n" + root_file.write_text(manual_content, encoding="utf-8") + + primitives = PrimitiveCollection() + primitives.add_primitive( + Instruction( + name="contributing", + file_path=temp_project / ".apm/instructions/contributing.instructions.md", + description="General contributing guidance", + apply_to="", + content="# Contributing\n\nRun focused tests first.", + author="test", + source="local", + ) + ) + + compiler = AgentsCompiler(str(temp_project)) + result = compiler.compile( + CompilationConfig(target="vscode", dry_run=True, single_agents=True), + primitives, + ) + + assert result.success + # Dry-run must not mutate the file. + assert root_file.read_text(encoding="utf-8") == manual_content + # Stats must mirror what a real run would record. + assert result.stats["copilot_root_instructions_generated"] == 0, ( + "dry-run must not claim generation when the real run would skip" + ) + assert result.stats["copilot_root_instructions_written"] == 0 + assert result.stats["copilot_root_instructions_skipped"] == 1, ( + "dry-run must surface the skip the real run would record" + ) + assert result.stats["copilot_root_instructions_unchanged"] == 0 + assert any( + "hand-authored file will not be overwritten" in w for w in result.warnings + ), "dry-run must surface the same warning a real run would emit" + + def test_generated_footer_uses_apm_compile_not_specify(self, temp_project): + """Generated footer must read `apm compile`, not `specify apm compile`.""" + primitives = PrimitiveCollection() + primitives.add_primitive( + Instruction( + name="contributing", + file_path=temp_project / ".apm/instructions/contributing.instructions.md", + description="General contributing guidance", + apply_to="", + content="# Contributing", + author="test", + source="local", + ) + ) + + compiler = AgentsCompiler(str(temp_project)) + result = compiler.compile( + CompilationConfig(target="vscode", dry_run=False, single_agents=True), + primitives, + ) + assert result.success + + content = (temp_project / ".github" / "copilot-instructions.md").read_text(encoding="utf-8") + assert "specify apm compile" not in content + assert "*To regenerate: `apm compile`*" in content + def test_unknown_target_returns_failure(self, temp_project, sample_primitives): """Unknown target must fail explicitly instead of silently succeeding.""" config = CompilationConfig( @@ -1187,11 +1494,17 @@ def test_single_string_passthrough(self): assert _resolve_compile_target("all") == "all" assert _resolve_compile_target("copilot") == "copilot" - def test_list_claude_and_copilot_returns_agents_claude_set(self): + def test_list_claude_and_copilot_returns_full_set(self): from apm_cli.commands.compile.cli import _resolve_compile_target - assert _resolve_compile_target(["claude", "vscode"]) == frozenset({"agents", "claude"}) - assert _resolve_compile_target(["claude", "copilot"]) == frozenset({"agents", "claude"}) + # 'vscode' family member is added because copilot/vscode/agents triggers + # copilot-instructions.md generation; 'agents' covers AGENTS.md. + assert _resolve_compile_target(["claude", "vscode"]) == frozenset( + {"vscode", "agents", "claude"} + ) + assert _resolve_compile_target(["claude", "copilot"]) == frozenset( + {"vscode", "agents", "claude"} + ) def test_list_claude_only_returns_claude(self): from apm_cli.commands.compile.cli import _resolve_compile_target @@ -1204,18 +1517,25 @@ def test_list_copilot_only_returns_vscode(self): assert _resolve_compile_target(["vscode"]) == "vscode" assert _resolve_compile_target(["copilot"]) == "vscode" - def test_list_agents_family_without_claude_returns_vscode(self): - """Targets that produce AGENTS.md but not CLAUDE.md.""" + def test_list_agents_md_only_family_preserves_bare_target(self): + """cursor/opencode/codex must NOT collapse to 'vscode' (which would + wrongly route copilot-instructions.md). Single-element lists keep the + bare target name; multi-element lists pick the first present.""" from apm_cli.commands.compile.cli import _resolve_compile_target - assert _resolve_compile_target(["cursor"]) == "vscode" - assert _resolve_compile_target(["opencode"]) == "vscode" - assert _resolve_compile_target(["codex"]) == "vscode" - assert _resolve_compile_target(["cursor", "opencode"]) == "vscode" + assert _resolve_compile_target(["cursor"]) == "cursor" + assert _resolve_compile_target(["opencode"]) == "opencode" + assert _resolve_compile_target(["codex"]) == "codex" + # Multi-element AGENTS.md-only list collapses to a representative bare + # target (cursor wins by deterministic ordering). + assert _resolve_compile_target(["cursor", "opencode"]) == "cursor" + assert _resolve_compile_target(["opencode", "codex"]) == "opencode" def test_list_cursor_and_claude_returns_agents_claude_set(self): from apm_cli.commands.compile.cli import _resolve_compile_target + # No 'vscode' family because copilot/vscode/agents was not requested; + # cursor/codex contribute only the 'agents' (AGENTS.md) family. assert _resolve_compile_target(["cursor", "claude"]) == frozenset({"agents", "claude"}) assert _resolve_compile_target(["codex", "claude"]) == frozenset({"agents", "claude"}) @@ -1229,19 +1549,21 @@ def test_list_gemini_and_claude_returns_claude_gemini_set(self): assert _resolve_compile_target(["gemini", "claude"]) == frozenset({"claude", "gemini"}) - def test_list_gemini_and_copilot_returns_agents_gemini_set(self): + def test_list_gemini_and_copilot_returns_full_set(self): from apm_cli.commands.compile.cli import _resolve_compile_target - assert _resolve_compile_target(["gemini", "vscode"]) == frozenset({"agents", "gemini"}) + assert _resolve_compile_target(["gemini", "vscode"]) == frozenset( + {"vscode", "agents", "gemini"} + ) def test_list_all_three_families_returns_full_set(self): from apm_cli.commands.compile.cli import _resolve_compile_target assert _resolve_compile_target(["claude", "vscode", "gemini"]) == frozenset( - {"agents", "claude", "gemini"} + {"vscode", "agents", "claude", "gemini"} ) assert _resolve_compile_target(["claude", "vscode", "cursor"]) == frozenset( - {"agents", "claude"} + {"vscode", "agents", "claude"} ) diff --git a/tests/unit/core/test_target_detection.py b/tests/unit/core/test_target_detection.py index 5df0286b2..c766274e0 100644 --- a/tests/unit/core/test_target_detection.py +++ b/tests/unit/core/test_target_detection.py @@ -261,6 +261,28 @@ def test_minimal_target(self): def test_claude_target(self): assert should_compile_copilot_instructions_md("claude") is False + def test_frozenset_with_vscode_returns_true(self): + """Multi-target lists containing 'vscode' family member must emit.""" + assert ( + should_compile_copilot_instructions_md(frozenset({"vscode", "agents", "claude"})) + is True + ) + + def test_frozenset_with_agents_only_returns_false(self): + """Multi-target lists that map cursor/opencode/codex to 'agents' + family for AGENTS.md routing must NOT trigger copilot-instructions.md. + + This is the round-3 regression: previously the predicate checked + '"agents" in target' which over-fired on cursor/opencode/codex combos. + """ + assert should_compile_copilot_instructions_md(frozenset({"agents", "claude"})) is False + assert should_compile_copilot_instructions_md(frozenset({"agents"})) is False + + def test_frozenset_without_vscode_returns_false(self): + """Multi-target lists without 'vscode' family must not emit.""" + assert should_compile_copilot_instructions_md(frozenset({"claude", "gemini"})) is False + assert should_compile_copilot_instructions_md(frozenset({"claude"})) is False + class TestGetTargetDescription: """Tests for get_target_description function."""