From 1547e78d42e216dd5fa374588ecff33f6d039f3e Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 30 Aug 2026 09:26:01 +0200 Subject: [PATCH 01/15] fix: preserve nested agent discovery Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 3 + .../instructions-and-agents.md | 20 +++- .../skills/apm-usage/package-authoring.md | 6 + src/apm_cli/deps/plugin_parser.py | 17 ++- src/apm_cli/integration/agent_integrator.py | 108 +++++++++++++++++- .../unit/integration/test_agent_integrator.py | 71 +++++++++--- tests/unit/test_plugin_parser.py | 25 ++++ 7 files changed, 220 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f0dd2347c..f9911e9ce1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm doctor` now reports malformed project executable-trust configuration under either `executables` or the deprecated `allowExecutables` key as an actionable informational warning instead of omitting the check. (#2719) +- Agent discovery now preserves nested agent paths, validates plain Markdown + definitions by `name` and `description` frontmatter, and warns when sibling + resources cannot be deployed. (closes #2692) - Git subdirectory dependencies with symlinks to files elsewhere in the same repository now install successfully where Git materializes symlinks; APM diff --git a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md index 0f442e8c38..cffe78c0d7 100644 --- a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md +++ b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md @@ -139,6 +139,14 @@ my-package/ File names end in `.agent.md` and live under `.apm/agents/`. +Agent definitions in nested directories keep that relative directory in the +target. Claude plugin manifests may also declare plain `.md` agent files; APM +accepts those only when they contain non-empty `name` and `description` fields +in YAML frontmatter. Other Markdown files and non-Markdown sibling resources +are not deployed as agents, and `apm install` lists them in a warning. If an +agent must ship scripts, templates, or other runtime resources, package it as a +skill bundle instead. + ### Frontmatter ```markdown @@ -209,12 +217,12 @@ offending package and field so you can fix the source. | Target | Output path | Transform | |---|---|---| -| copilot | `.github/agents/.agent.md` | verbatim | -| claude | `.claude/agents/.md` | verbatim | -| grok-build | `.grok/agents/.md` | verbatim | -| cursor | `.cursor/agents/.md` | verbatim | -| opencode | `.opencode/agents/.md` | verbatim | -| codex | `.codex/agents/.toml` | `name` and `description` -> TOML; body becomes `developer_instructions`; unsupported `tools` emits a warning | +| copilot | `.github/agents/.agent.md` | verbatim | +| claude | `.claude/agents/.md` | verbatim | +| grok-build | `.grok/agents/.md` | verbatim | +| cursor | `.cursor/agents/.md` | verbatim | +| opencode | `.opencode/agents/.md` | verbatim | +| codex | `.codex/agents/.toml` | `name` and `description` -> TOML; body becomes `developer_instructions`; unsupported `tools` emits a warning | | kiro | `.kiro/agents/.md` | `description`, `model`, `tools` kept; `name` and unknown fields stripped; identity from path; fail closed on unsupported tools (ref: [kiro.dev/docs/custom-agents](https://kiro.dev/docs/custom-agents/), accessed 2026-08-03) | | grok-build | `.grok/agents/.md` | verbatim | | windsurf | not deployed | Windsurf has no agents primitive -- author personas as skills (Cascade auto-invokes by description) | diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 6aba33e1c1..a8d033c77f 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -341,6 +341,12 @@ supported top-level harness directories: `.agents`, `.apm`, `.claude`, Chat persona configuration. Place in `.apm/agents/`. +Nested agent definitions preserve their relative directory when installed. +Plain `.md` definitions from Claude plugins must declare non-empty `name` and +`description` fields in YAML frontmatter; other Markdown and non-Markdown files +under the agent directory are skipped with an install warning. Use a skill +bundle when the runtime needs sibling scripts, templates, or other resources. + ```yaml --- name: "architect" diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py index 795b33455e..8cd217ddbf 100644 --- a/src/apm_cli/deps/plugin_parser.py +++ b/src/apm_cli/deps/plugin_parser.py @@ -1148,21 +1148,28 @@ def _is_same_path(src: Path, dst: Path) -> bool: return False # Map agents/ - # Unlike skills (which are named directories containing SKILL.md), agents - # are flat files -- each .md is one agent. So we always merge directory - # contents directly into .apm/agents/ (no nesting by dir name). + # The top-level agents/ directory is a collection, so merge its contents + # directly into .apm/agents/. A manifest may instead declare one of its + # child directories as an agent bundle. Preserve that relative directory + # so nested agent identity and sibling resources do not get flattened. agent_sources = _resolve_sources("agents", "agents") if agent_sources: target_agents = apm_dir / "agents" + default_agents = (plugin_path / "agents").resolve() _assert_no_symlink_descendants(target_agents) agent_dirs = [s for s in agent_sources if s.is_dir()] agent_files = [s for s in agent_sources if s.is_file()] for d in agent_dirs: - if _is_same_path(d, target_agents): + try: + relative_bundle = d.relative_to(default_agents) + except ValueError: + relative_bundle = Path() + destination = target_agents / relative_bundle + if _is_same_path(d, destination): continue shutil.copytree( d, - target_agents, + destination, dirs_exist_ok=True, ignore=ignore_non_content, ) diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index 2843468514..eb015c64a0 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -16,6 +16,7 @@ from apm_cli.integration.base_integrator import BaseIntegrator, IntegrationResult from apm_cli.integration.opencode_frontmatter import validate_opencode_frontmatter from apm_cli.utils.atomic_io import normalize_crlf_to_lf, write_text_lf +from apm_cli.utils.console import _rich_warning from apm_cli.utils.diagnostics import printable_ascii_text from apm_cli.utils.path_security import PathTraversalError, ensure_path_within from apm_cli.utils.paths import portable_relpath @@ -56,7 +57,8 @@ def find_agent_files(self, package_path: Path, source_plan=None) -> list[Path]: Searches in: - Package root directory (*.agent.md files) - - .apm/agents/ subdirectory (recursive): *.agent.md and plain *.md files + - .apm/agents/ subdirectory (recursive): explicit *.agent.md files + and plain *.md files with agent frontmatter Args: package_path: Path to the package directory @@ -71,12 +73,87 @@ def find_agent_files(self, package_path: Path, source_plan=None) -> list[Path]: apm_agents = package_path / ".apm" / "agents" if apm_agents.exists(): files += self.find_files_by_glob(apm_agents, "**/*.agent.md") - # Also pick up plain .md files; the directory name implies type + # Claude plugin agents may use plain .md names. Require their + # mandatory description frontmatter so guides and templates are + # not exposed to the target runtime as invokable agents. for f in self.find_files_by_glob(apm_agents, "**/*.md"): - if not f.name.endswith(".agent.md") and f not in files: + if not f.name.endswith(".agent.md") and self._is_plain_md_agent(f): files.append(f) return self.filter_authorized_files(files, source_plan) + @staticmethod + def _is_plain_md_agent(source: Path) -> bool: + """Return whether a plain Markdown file declares agent frontmatter.""" + try: + content = source.read_text(encoding="utf-8") + except (OSError, UnicodeError): + return False + match = AgentIntegrator._FRONTMATTER_RE.match(content) + if match is None: + return False + try: + frontmatter = load_yaml_str(match.group(1)) + except yaml.YAMLError: + return False + if not isinstance(frontmatter, dict): + return False + name = frontmatter.get("name") + description = frontmatter.get("description") + return ( + isinstance(name, str) + and bool(name.strip()) + and isinstance(description, str) + and bool(description.strip()) + ) + + def _warn_ignored_agent_resources( + self, + package_path: Path, + package_name: str, + diagnostics=None, + source_plan=None, + ) -> None: + """Warn when files under .apm/agents are not deployable agents.""" + apm_agents = package_path / ".apm" / "agents" + if not apm_agents.is_dir(): + return + candidates = [ + path + for path in self.find_files_by_glob(apm_agents, "**/*") + if path.is_file() + and not path.name.endswith(".agent.md") + and not (path.suffix == ".md" and self._is_plain_md_agent(path)) + ] + ignored = self.filter_authorized_files(candidates, source_plan) + if not ignored: + return + relative = sorted(portable_relpath(path, package_path) for path in ignored) + message = ( + f"Ignored {len(relative)} non-agent file(s) under .apm/agents; " + "only *.agent.md files and plain Markdown files with name and " + "description frontmatter are deployable." + ) + detail = ", ".join(relative) + if diagnostics is not None: + diagnostics.warn(message=message, package=package_name, detail=detail) + else: + _rich_warning(f"{message} Files: {detail}") + + @staticmethod + def _source_agent_relpath(source_file: Path, package_path: Path | None = None) -> Path: + """Return an agent's path relative to the canonical agents directory.""" + if package_path is not None: + try: + return source_file.relative_to(package_path / ".apm" / "agents") + except ValueError: + return Path(source_file.name) + + parts = source_file.parts + for index in range(len(parts) - 1): + if parts[index : index + 2] == (".apm", "agents"): + return Path(*parts[index + 2 :]) + return Path(source_file.name) + # NOTE: find_skill_file(), integrate_skill(), and _generate_skill_agent_content() # have been REMOVED as part of T5 (skill-strategy.md). # @@ -94,12 +171,14 @@ def get_target_filename_for_target( source_file: Path, package_name: str, target: TargetProfile, + package_path: Path | None = None, ) -> str: - """Generate target filename using the extension from *target*'s agents mapping.""" + """Generate a target-relative path using the target agent extension.""" mapping = target.primitives.get("agents") ext = mapping.extension if mapping else ".agent.md" stem = source_file.name[:-9] if source_file.name.endswith(".agent.md") else source_file.stem - return f"{stem}{ext}" + source_relpath = self._source_agent_relpath(source_file, package_path) + return (source_relpath.parent / f"{stem}{ext}").as_posix() def integrate_agents_for_target( self, @@ -130,6 +209,12 @@ def integrate_agents_for_target( self.init_link_resolver(package_info, project_root) agent_files = self.find_agent_files(package_info.install_path, source_plan) + self._warn_ignored_agent_resources( + package_info.install_path, + package_info.package.name, + diagnostics, + source_plan, + ) if not agent_files: return IntegrationResult(0, 0, 0, []) @@ -153,6 +238,7 @@ def integrate_agents_for_target( source_file, package_info.package.name, target, + package_info.install_path, ) target_path = agents_dir / target_relpath # Defense-in-depth: assert containment under agents_dir so a @@ -231,6 +317,7 @@ def integrate_agents_for_target( files_skipped += 1 continue + target_path.parent.mkdir(parents=True, exist_ok=True) if mapping.format_id == "codex_agent": self._write_codex_agent( source_file, @@ -669,6 +756,11 @@ def integrate_package_agents( self.init_link_resolver(package_info, project_root) agent_files = self.find_agent_files(package_info.install_path) + self._warn_ignored_agent_resources( + package_info.install_path, + package_info.package.name, + diagnostics, + ) if not agent_files: return IntegrationResult(0, 0, 0, []) @@ -698,6 +790,7 @@ def integrate_package_agents( source_file, package_info.package.name, copilot, + package_info.install_path, ) target_path = agents_dir / target_filename try: @@ -722,6 +815,7 @@ def integrate_package_agents( ): files_skipped += 1 continue + target_path.parent.mkdir(parents=True, exist_ok=True) links_resolved = self.copy_agent(source_file, target_path) total_links_resolved += links_resolved files_integrated += 1 @@ -733,6 +827,7 @@ def integrate_package_agents( source_file, package_info.package.name, claude_target, + package_info.install_path, ) claude_path = claude_agents_dir / claude_filename try: @@ -752,6 +847,7 @@ def integrate_package_agents( elif not self.check_collision( claude_path, claude_rel, managed_files, force, diagnostics=diagnostics ): + claude_path.parent.mkdir(parents=True, exist_ok=True) self.copy_agent(source_file, claude_path) target_paths.append(claude_path) @@ -761,6 +857,7 @@ def integrate_package_agents( source_file, package_info.package.name, cursor_target, + package_info.install_path, ) cursor_path = cursor_agents_dir / cursor_filename try: @@ -780,6 +877,7 @@ def integrate_package_agents( elif not self.check_collision( cursor_path, cursor_rel, managed_files, force, diagnostics=diagnostics ): + cursor_path.parent.mkdir(parents=True, exist_ok=True) self.copy_agent(source_file, cursor_path) target_paths.append(cursor_path) diff --git a/tests/unit/integration/test_agent_integrator.py b/tests/unit/integration/test_agent_integrator.py index 4cc98ced7a..7621ca85fe 100644 --- a/tests/unit/integration/test_agent_integrator.py +++ b/tests/unit/integration/test_agent_integrator.py @@ -508,15 +508,18 @@ def test_find_agent_files_ignores_skill_files(self): assert "SKILL.md" not in found_names assert "skill.md" not in found_names - def test_find_agent_files_includes_all_md(self): - """All .md files in .apm/agents/ are discovered — the directory - already implies type, so no name-based filtering.""" + def test_find_agent_files_requires_frontmatter_for_plain_md(self): + """Plain Markdown needs agent frontmatter to be an agent.""" package_dir = self.project_root / "package" apm_agents = package_dir / ".apm" / "agents" apm_agents.mkdir(parents=True) - (apm_agents / "planner.md").write_text("# Planner agent") - (apm_agents / "coder.md").write_text("# Coder agent") + (apm_agents / "planner.md").write_text( + "---\nname: planner\ndescription: Plans implementation work\n---\n# Planner agent" + ) + (apm_agents / "coder.md").write_text( + "---\nname: coder\ndescription: Implements approved plans\n---\n# Coder agent" + ) (apm_agents / "README.md").write_text("# Docs") (apm_agents / "CHANGELOG.md").write_text("# Changes") (apm_agents / "LICENSE.md").write_text("MIT") @@ -525,14 +528,7 @@ def test_find_agent_files_includes_all_md(self): agents = self.integrator.find_agent_files(package_dir) names = {a.name for a in agents} - assert names == { - "planner.md", - "coder.md", - "README.md", - "CHANGELOG.md", - "LICENSE.md", - "CONTRIBUTING.md", - } + assert names == {"planner.md", "coder.md"} def test_find_agent_files_discovers_nested_subdirectories(self): """find_agent_files uses rglob so agents in subdirs are found.""" @@ -543,7 +539,9 @@ def test_find_agent_files_discovers_nested_subdirectories(self): (apm_agents / "top-level.agent.md").write_text("# Top") (nested / "nested.agent.md").write_text("# Nested agent.md") - (nested / "plain-nested.md").write_text("# Nested plain") + (nested / "plain-nested.md").write_text( + "---\nname: plain-nested\ndescription: Nested plain agent\n---\n# Nested plain" + ) agents = self.integrator.find_agent_files(package_dir) names = {a.name for a in agents} @@ -552,6 +550,51 @@ def test_find_agent_files_discovers_nested_subdirectories(self): assert "nested.agent.md" in names assert "plain-nested.md" in names + def test_nested_agent_bundle_filters_resources_and_preserves_agent_path(self): + """Nested resources are not agents and nested agent identity is preserved.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + package_dir = self.project_root / "package" + agent_dir = package_dir / ".apm" / "agents" / "my-agent" + (agent_dir / "guides").mkdir(parents=True) + (agent_dir / "scripts").mkdir() + (agent_dir / "my-agent.md").write_text( + "---\nname: my-agent\ndescription: Test agent\n---\nUse scripts/helper.py.\n" + ) + (agent_dir / "guides" / "reference-doc.md").write_text("# Reference\n") + (agent_dir / "scripts" / "helper.py").write_text("print('helper')\n") + + package = APMPackage(name="test-pkg", version="1.0.0", package_path=package_dir) + package_info = PackageInfo( + package=package, + install_path=package_dir, + resolved_reference=ResolvedReference( + original_ref="main", + ref_type=GitReferenceType.BRANCH, + resolved_commit="abc123", + ref_name="main", + ), + installed_at=datetime.now().isoformat(), + ) + diagnostics = DiagnosticCollector() + + result = self.integrator.integrate_agents_for_target( + KNOWN_TARGETS["copilot"], + package_info, + self.project_root, + diagnostics=diagnostics, + ) + + expected = self.project_root / ".github" / "agents" / "my-agent" / "my-agent.agent.md" + assert result.target_paths == [expected] + assert expected.is_file() + assert not (self.project_root / ".github" / "agents" / "reference-doc.agent.md").exists() + warnings = diagnostics.by_category().get(CATEGORY_WARNING, []) + assert len(warnings) == 1 + assert warnings[0].detail == ( + ".apm/agents/my-agent/guides/reference-doc.md, .apm/agents/my-agent/scripts/helper.py" + ) + def test_get_target_filename_plain_md(self): """Plain .md files get renamed to .agent.md for .github/agents/.""" source = Path("/package/.apm/agents/context-architect.md") diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py index bb6cebbc27..44c63b2c47 100644 --- a/tests/unit/test_plugin_parser.py +++ b/tests/unit/test_plugin_parser.py @@ -843,6 +843,31 @@ def test_custom_agents_dir_list_flattens_contents(self, tmp_path): "Should not create nested agents/agents/ directory" ) + def test_declared_agent_subdirectory_preserves_bundle_path(self, tmp_path): + """A declared directory under agents keeps its grouping and resources.""" + plugin_dir = tmp_path / "plugin" + agent_dir = plugin_dir / "agents" / "my-agent" + (agent_dir / "guides").mkdir(parents=True) + (agent_dir / "scripts").mkdir() + (agent_dir / "my-agent.md").write_text( + "---\nname: my-agent\ndescription: Test agent\n---\nUse scripts/helper.py.\n" + ) + (agent_dir / "guides" / "reference-doc.md").write_text("# Reference\n") + (agent_dir / "scripts" / "helper.py").write_text("print('helper')\n") + + apm_dir = plugin_dir / ".apm" + apm_dir.mkdir() + _map_plugin_artifacts( + plugin_dir, + apm_dir, + manifest={"agents": ["./agents/my-agent"]}, + ) + + staged = apm_dir / "agents" / "my-agent" + assert (staged / "my-agent.md").is_file() + assert (staged / "guides" / "reference-doc.md").is_file() + assert (staged / "scripts" / "helper.py").is_file() + class TestGenerateApmYml: def test_generate_full_metadata(self): From 02bbe0c78c35aabcad8f04c86d6ecd466d2b9a5b Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 30 Aug 2026 12:14:16 +0200 Subject: [PATCH 02/15] fix: harden nested agent inventory Prepare agent sources once per package, preserve warning coverage outside the deployable source plan, and sanitize package-controlled diagnostics. Add regression and architecture guardrails to address the review panel follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../owners/install-deployment.json | 7 ++ .../checks/install_agent_inventory.py | 59 ++++++++++++ .../checks/install_deployment_analyzers.py | 9 ++ scripts/architecture_linter/diagnostics.py | 1 + src/apm_cli/install/services.py | 32 +++++++ src/apm_cli/integration/agent_integrator.py | 93 +++++++++++-------- .../test_architecture_owner_rule_mutations.py | 10 +- .../test_install_services_orchestration.py | 27 ++++++ .../test_integrators_validation_rules.py | 10 +- .../test_marketplace_plugin_integration.py | 71 ++++++++++++++ .../unit/integration/test_agent_integrator.py | 31 +++++++ 11 files changed, 309 insertions(+), 41 deletions(-) create mode 100644 scripts/architecture_linter/checks/install_agent_inventory.py diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index 5fa44ca3da..25528c83fa 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -50,6 +50,13 @@ "selectors": ["src/apm_cli/install/service.py"], "guards": ["install-deployment-frozen-mutation-eligibility"] }, + { + "id": "agent-source-admission-inventory", + "decision": "Agent source admission, relative identity, and package-level inventory", + "owner": "integration/agent_integrator.py (prepare_agent_files, _is_plain_md_agent, _source_agent_relpath)", + "selectors": ["src/apm_cli/integration/agent_integrator.py"], + "guards": ["install-deployment-agent-source-inventory"] + }, { "id": "authorized-deployable-source-paths", "decision": "Authorized deployable source paths", diff --git a/scripts/architecture_linter/checks/install_agent_inventory.py b/scripts/architecture_linter/checks/install_agent_inventory.py new file mode 100644 index 0000000000..fbffb70011 --- /dev/null +++ b/scripts/architecture_linter/checks/install_agent_inventory.py @@ -0,0 +1,59 @@ +"""Agent source admission and package-inventory architecture guard.""" + +from __future__ import annotations + +from scripts.architecture_linter.checks.install_deployment_shared import ( + _facts_for, + _present, + _python_paths, + _summary, +) +from scripts.architecture_linter.facts import FactsProvider +from scripts.architecture_linter.models import Violation + +_GUARD_AGENT_SOURCE_INVENTORY = "install-deployment-agent-source-inventory" +_OWNER = "src/apm_cli/integration/agent_integrator.py" +_SERVICES = "src/apm_cli/install/services.py" +_OWNER_DEFINITIONS = frozenset( + {"_is_plain_md_agent", "_source_agent_relpath", "prepare_agent_files"} +) + + +def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, ...]: + """Agent admission, identity, and inventory must route through AgentIntegrator.""" + rule_id = _GUARD_AGENT_SOURCE_INVENTORY + owner, owner_fail = _facts_for(provider, _OWNER, rule_id) + services, services_fail = _facts_for(provider, _SERVICES, rule_id) + if owner_fail or services_fail: + return tuple(list(owner_fail) + list(services_fail)) + + definition_counts = dict.fromkeys(_OWNER_DEFINITIONS, 0) + for path in _python_paths(provider, "src/apm_cli/"): + facts, failures = _facts_for(provider, path, rule_id) + if failures: + return tuple(failures) + for definition in facts.definitions: + if definition.name in definition_counts: + definition_counts[definition.name] += 1 + + required_owner_fragments = ( + "files, _ignored = self._classify_agent_files(package_path)", + "agent_files, ignored_resources = self._classify_agent_files(package_path)", + "if agent_files is None:", + ) + if ( + any(count != 1 for count in definition_counts.values()) + or any(not _present(owner, fragment) for fragment in required_owner_fragments) + or not _present(services, '"agent_files": integrator.prepare_agent_files(') + ): + return ( + _summary( + rule_id, + _OWNER, + "Agent admission, relative identity, and inventory must route through AgentIntegrator", + ), + ) + return () + + +__all__ = ["_GUARD_AGENT_SOURCE_INVENTORY", "check_agent_source_inventory"] diff --git a/scripts/architecture_linter/checks/install_deployment_analyzers.py b/scripts/architecture_linter/checks/install_deployment_analyzers.py index 3acdc3237e..1237dff7d9 100644 --- a/scripts/architecture_linter/checks/install_deployment_analyzers.py +++ b/scripts/architecture_linter/checks/install_deployment_analyzers.py @@ -13,6 +13,10 @@ from __future__ import annotations +from scripts.architecture_linter.checks.install_agent_inventory import ( + _GUARD_AGENT_SOURCE_INVENTORY, + check_agent_source_inventory, +) from scripts.architecture_linter.checks.install_base_integrator_and_contraction import ( _GUARD_BASE_INTEGRATOR, _GUARD_PROVENANCE, @@ -54,6 +58,11 @@ from scripts.architecture_linter.models import Rule RULES: tuple[Rule, ...] = ( + _rule( + _GUARD_AGENT_SOURCE_INVENTORY, + "Agent admission, relative identity, and inventory stay owned by AgentIntegrator.", + check_agent_source_inventory, + ), _rule( _GUARD_PACKAGE_TARGET, "Restriction-only package target authorization has one owner (install/target_filter.py).", diff --git a/scripts/architecture_linter/diagnostics.py b/scripts/architecture_linter/diagnostics.py index b74a4d6be9..f0f4c6bfe3 100644 --- a/scripts/architecture_linter/diagnostics.py +++ b/scripts/architecture_linter/diagnostics.py @@ -36,6 +36,7 @@ "contracts-tooling-dependency-identity": ("AC23", "AC25", "AC29"), "contracts-tooling-frontmatter-yaml": ("AC36",), "install-deployment-approval-outcome-routing": ("AC3",), + "install-deployment-agent-source-inventory": ("AC37",), "install-deployment-audit-policy-discovery": ("AC3",), "install-deployment-audit-replay": ("AC4",), "install-deployment-cached-claude-skill-metadata": ("AC4",), diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 168c6d0ddd..bd525f6ad6 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -214,6 +214,29 @@ def _log_package_target_restriction(logger: InstallLogger | None, target_selecti ) +def _prepare_primitive_inputs( + primitive_name: str, + integrator: Any, + package_info: Any, + targets: Any, + diagnostics: Any, + source_plan: Any, +) -> dict[str, Any]: + """Prepare package-scoped inputs reused across target integrations.""" + if primitive_name != "agents" or not any( + target.primitives.get("agents") is not None for target in targets + ): + return {} + return { + "agent_files": integrator.prepare_agent_files( + package_info.install_path, + package_info.package.name, + diagnostics, + source_plan, + ) + } + + def integrate_package_primitives( # noqa: PLR0913 package_info: Any, project_root: Path, @@ -500,6 +523,14 @@ def _format_target_collapse(paths: list[str], verbose: bool) -> tuple[str, list[ _agg_paths: list[str] = [] _agg_hook_payloads: list = [] _label = _prim_name + _prepared_inputs = _prepare_primitive_inputs( + _prim_name, + _integrator, + package_info, + targets, + diagnostics, + source_plan, + ) for _target in targets: _mapping = _target.primitives.get(_prim_name) if _mapping is None: @@ -510,6 +541,7 @@ def _format_target_collapse(paths: list[str], verbose: bool) -> tuple[str, list[ "diagnostics": diagnostics, "scope": scope, "source_plan": source_plan, + **_prepared_inputs, } # Hook integrator alone needs the scope signal: project-scope # deploys keep ``command`` paths repo-relative (#1394), user-scope diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index eb015c64a0..a419e7aedd 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -66,20 +66,25 @@ def find_agent_files(self, package_path: Path, source_plan=None) -> list[Path]: Returns: List[Path]: List of absolute paths to agent files """ - files: list[Path] = [] - # Flat search in package root - files += self.find_files_by_glob(package_path, "*.agent.md") - # Recursive search in .apm/agents/ (use ** glob for subdirectories) + files, _ignored = self._classify_agent_files(package_path) + return self.filter_authorized_files(files, source_plan) + + def _classify_agent_files(self, package_path: Path) -> tuple[list[Path], list[Path]]: + """Classify package agent files and ignored sibling resources once.""" + files = self.find_files_by_glob(package_path, "*.agent.md") + ignored: list[Path] = [] apm_agents = package_path / ".apm" / "agents" if apm_agents.exists(): - files += self.find_files_by_glob(apm_agents, "**/*.agent.md") - # Claude plugin agents may use plain .md names. Require their - # mandatory description frontmatter so guides and templates are - # not exposed to the target runtime as invokable agents. - for f in self.find_files_by_glob(apm_agents, "**/*.md"): - if not f.name.endswith(".agent.md") and self._is_plain_md_agent(f): - files.append(f) - return self.filter_authorized_files(files, source_plan) + for path in self.find_files_by_glob(apm_agents, "**/*"): + if not path.is_file(): + continue + if path.name.endswith(".agent.md") or ( + path.suffix == ".md" and self._is_plain_md_agent(path) + ): + files.append(path) + else: + ignored.append(path) + return files, ignored @staticmethod def _is_plain_md_agent(source: Path) -> bool: @@ -110,35 +115,49 @@ def _warn_ignored_agent_resources( self, package_path: Path, package_name: str, + ignored_resources: list[Path], diagnostics=None, - source_plan=None, ) -> None: """Warn when files under .apm/agents are not deployable agents.""" - apm_agents = package_path / ".apm" / "agents" - if not apm_agents.is_dir(): - return - candidates = [ - path - for path in self.find_files_by_glob(apm_agents, "**/*") - if path.is_file() - and not path.name.endswith(".agent.md") - and not (path.suffix == ".md" and self._is_plain_md_agent(path)) - ] - ignored = self.filter_authorized_files(candidates, source_plan) - if not ignored: + if not ignored_resources: return - relative = sorted(portable_relpath(path, package_path) for path in ignored) + relative = sorted( + printable_ascii_text(portable_relpath(path, package_path)) for path in ignored_resources + ) message = ( f"Ignored {len(relative)} non-agent file(s) under .apm/agents; " "only *.agent.md files and plain Markdown files with name and " - "description frontmatter are deployable." + "description frontmatter are deployable. Run with --verbose to list " + "the files. Package required runtime resources as a skill bundle, " + "then rerun 'apm install'." ) detail = ", ".join(relative) if diagnostics is not None: - diagnostics.warn(message=message, package=package_name, detail=detail) + diagnostics.warn( + message=message, + package=printable_ascii_text(package_name), + detail=detail, + ) else: _rich_warning(f"{message} Files: {detail}") + def prepare_agent_files( + self, + package_path: Path, + package_name: str, + diagnostics=None, + source_plan=None, + ) -> list[Path]: + """Discover deployable agents and report ignored resources once.""" + agent_files, ignored_resources = self._classify_agent_files(package_path) + self._warn_ignored_agent_resources( + package_path, + package_name, + ignored_resources, + diagnostics, + ) + return self.filter_authorized_files(agent_files, source_plan) + @staticmethod def _source_agent_relpath(source_file: Path, package_path: Path | None = None) -> Path: """Return an agent's path relative to the canonical agents directory.""" @@ -191,6 +210,7 @@ def integrate_agents_for_target( diagnostics=None, scope=None, source_plan=None, + agent_files: list[Path] | None = None, ) -> IntegrationResult: """Integrate agents from a package for a single *target*. @@ -208,13 +228,13 @@ def integrate_agents_for_target( return IntegrationResult(0, 0, 0, []) self.init_link_resolver(package_info, project_root) - agent_files = self.find_agent_files(package_info.install_path, source_plan) - self._warn_ignored_agent_resources( - package_info.install_path, - package_info.package.name, - diagnostics, - source_plan, - ) + if agent_files is None: + agent_files = self.prepare_agent_files( + package_info.install_path, + package_info.package.name, + diagnostics, + source_plan, + ) if not agent_files: return IntegrationResult(0, 0, 0, []) @@ -755,8 +775,7 @@ def integrate_package_agents( copilot = KNOWN_TARGETS["copilot"] self.init_link_resolver(package_info, project_root) - agent_files = self.find_agent_files(package_info.install_path) - self._warn_ignored_agent_resources( + agent_files = self.prepare_agent_files( package_info.install_path, package_info.package.name, diagnostics, diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index f144e2f470..1247c55ef8 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -6,7 +6,7 @@ every guard executes exactly once per run. Names prove nothing about teeth: a rule whose body was gutted still registers its guard ID and still runs. -This file supplies the missing half of that contract. For each of the 55 +This file supplies the missing half of that contract. For each of the 56 registered owner guards it pins one minimal, meaningful source mutation -- a surgical edit that kills a load-bearing sub-condition of the owning decision -- and asserts the one rule that owns that guard reports a real `Violation`. @@ -203,6 +203,14 @@ class MutationCase: new="include_scoped_in_user_root_context: bool = True", intent="TargetProfile flips the user-root scoped-instruction eligibility default.", ), + MutationCase( + guard_id="install-deployment-agent-source-inventory", + rule_id="install-deployment-agent-source-inventory", + path="src/apm_cli/integration/agent_integrator.py", + old=" def prepare_agent_files(", + new=" def prepare_agent_files_disabled(", + intent="AgentIntegrator loses its canonical package-level inventory entry point.", + ), MutationCase( guard_id="install-deployment-audit-replay", rule_id="install-deployment-audit-replay", diff --git a/tests/integration/test_install_services_orchestration.py b/tests/integration/test_install_services_orchestration.py index d43f8e104e..5097492190 100644 --- a/tests/integration/test_install_services_orchestration.py +++ b/tests/integration/test_install_services_orchestration.py @@ -418,6 +418,33 @@ def test_dispatch_calls_agent_integrator(self, tmp_path: Path) -> None: integrators["agent_integrator"].integrate_agents_for_target.assert_called_once() assert result["agents"] == 1 + def test_agent_files_are_prepared_once_for_multiple_targets(self, tmp_path: Path) -> None: + targets = [ + make_target(name="copilot", primitives={"agents": make_mapping(subdir="agents")}), + make_target(name="claude", primitives={"agents": make_mapping(subdir="agents")}), + ] + entry = make_dispatch_entry( + integrate_method="integrate_agents_for_target", + counter_key="agents", + ) + + _, integrators, diagnostics, _ = invoke_integrate( + tmp_path, + targets=targets, + dispatch_table={"agents": entry}, + integrator_results={"agents": make_integration_result(files_integrated=1)}, + ) + + agent_integrator = integrators["agent_integrator"] + agent_integrator.prepare_agent_files.assert_called_once() + assert agent_integrator.integrate_agents_for_target.call_count == 2 + prepared = agent_integrator.prepare_agent_files.return_value + assert all( + call.kwargs["agent_files"] is prepared + for call in agent_integrator.integrate_agents_for_target.call_args_list + ) + assert agent_integrator.prepare_agent_files.call_args.args[2] is diagnostics + def test_instruction_cursor_rules_use_rule_label(self, tmp_path: Path) -> None: target = make_target( primitives={ diff --git a/tests/integration/test_integrators_validation_rules.py b/tests/integration/test_integrators_validation_rules.py index f643cf1fce..e2a542ae9f 100644 --- a/tests/integration/test_integrators_validation_rules.py +++ b/tests/integration/test_integrators_validation_rules.py @@ -453,13 +453,17 @@ def test_finds_agent_md_in_apm_agents_subdir(self, tmp_path: Path) -> None: assert any(f.name == "reviewer.agent.md" for f in files) def test_finds_plain_md_in_apm_agents_subdir(self, tmp_path: Path) -> None: - """Plain .md files in .apm/agents/ are also included.""" + """Plain .md files need agent frontmatter.""" apm_agents = tmp_path / ".apm" / "agents" apm_agents.mkdir(parents=True) - (apm_agents / "helper.md").write_text("# Helper") + (apm_agents / "helper.md").write_text( + "---\nname: helper\ndescription: Helps with tasks\n---\n# Helper" + ) + (apm_agents / "missing-description.md").write_text("---\nname: helper\n---\n# Helper") integrator = AgentIntegrator() files = integrator.find_agent_files(tmp_path) - assert any(f.name == "helper.md" for f in files) + names = {file.name for file in files} + assert names == {"helper.md"} def test_finds_chatmode_in_apm_chatmodes_subdir(self, tmp_path: Path) -> None: apm_chatmodes = tmp_path / ".apm" / "agents" diff --git a/tests/integration/test_marketplace_plugin_integration.py b/tests/integration/test_marketplace_plugin_integration.py index fbff24ee8d..957fdc577b 100644 --- a/tests/integration/test_marketplace_plugin_integration.py +++ b/tests/integration/test_marketplace_plugin_integration.py @@ -16,10 +16,14 @@ from click.testing import CliRunner from apm_cli.commands.install import install +from apm_cli.deps.plugin_parser import _map_plugin_artifacts +from apm_cli.install.deployable_source_plan import DeployableSourcePlan from apm_cli.integration.agent_integrator import AgentIntegrator from apm_cli.integration.command_integrator import CommandIntegrator from apm_cli.integration.prompt_integrator import PromptIntegrator from apm_cli.integration.skill_integrator import SkillIntegrator +from apm_cli.integration.targets import KNOWN_TARGETS +from apm_cli.utils.diagnostics import CATEGORY_WARNING, DiagnosticCollector from src.apm_cli.models.apm_package import ( APMPackage, GitReferenceType, @@ -236,6 +240,73 @@ def test_plugin_detection_and_structure_mapping(self, tmp_path): "Command should be mapped to prompts" ) + def test_nested_agent_bundle_maps_and_deploys_to_multiple_targets(self, tmp_path): + """A declared agent bundle keeps identity and reports sibling resources.""" + plugin_dir = tmp_path / "plugin" + agent_dir = plugin_dir / "agents" / "my-agent" + (agent_dir / "scripts").mkdir(parents=True) + (agent_dir / "my-agent.md").write_text( + "---\nname: my-agent\ndescription: Test agent\n---\n# Agent\n" + ) + (agent_dir / "scripts" / "helper.py").write_text("print('helper')\n") + apm_dir = plugin_dir / ".apm" + apm_dir.mkdir() + _map_plugin_artifacts( + plugin_dir, + apm_dir, + manifest={"agents": ["./agents/my-agent"]}, + ) + + package = APMPackage(name="test-pkg", version="1.0.0", package_path=plugin_dir) + package_info = PackageInfo( + package=package, + install_path=plugin_dir, + resolved_reference=ResolvedReference( + original_ref="main", + ref_type=GitReferenceType.BRANCH, + resolved_commit="abc123", + ref_name="main", + ), + installed_at=datetime.now().isoformat(), + ) + targets = [KNOWN_TARGETS["copilot"], KNOWN_TARGETS["claude"]] + (tmp_path / ".claude").mkdir() + source_plan = DeployableSourcePlan.create( + package_info, + targets, + skill_subset=None, + hooks_approved=False, + canvas_approved=False, + skip_bin=True, + ) + diagnostics = DiagnosticCollector() + integrator = AgentIntegrator() + agent_files = integrator.prepare_agent_files( + plugin_dir, + package.name, + diagnostics, + source_plan, + ) + + results = [ + integrator.integrate_agents_for_target( + target, + package_info, + tmp_path, + diagnostics=diagnostics, + source_plan=source_plan, + agent_files=agent_files, + ) + for target in targets + ] + + assert (tmp_path / ".github/agents/my-agent/my-agent.agent.md").is_file() + assert (tmp_path / ".claude/agents/my-agent/my-agent.md").is_file() + assert sum(result.files_integrated for result in results) == 2 + warnings = diagnostics.by_category()[CATEGORY_WARNING] + assert len(warnings) == 1 + assert warnings[0].detail == ".apm/agents/my-agent/scripts/helper.py" + def test_plugin_with_dependencies(self, tmp_path): """Test plugin with dependencies are handled correctly.""" plugin_dir = tmp_path / "plugin-with-deps" diff --git a/tests/unit/integration/test_agent_integrator.py b/tests/unit/integration/test_agent_integrator.py index 7621ca85fe..de4beafe1c 100644 --- a/tests/unit/integration/test_agent_integrator.py +++ b/tests/unit/integration/test_agent_integrator.py @@ -5,6 +5,7 @@ from pathlib import Path from unittest.mock import Mock +from apm_cli.install.deployable_source_plan import DeployableSourcePlan from apm_cli.integration import AgentIntegrator from apm_cli.models.apm_package import APMPackage, GitReferenceType, PackageInfo, ResolvedReference from apm_cli.utils.diagnostics import ( @@ -577,12 +578,21 @@ def test_nested_agent_bundle_filters_resources_and_preserves_agent_path(self): installed_at=datetime.now().isoformat(), ) diagnostics = DiagnosticCollector() + source_plan = DeployableSourcePlan.create( + package_info, + [KNOWN_TARGETS["copilot"]], + skill_subset=None, + hooks_approved=False, + canvas_approved=False, + skip_bin=True, + ) result = self.integrator.integrate_agents_for_target( KNOWN_TARGETS["copilot"], package_info, self.project_root, diagnostics=diagnostics, + source_plan=source_plan, ) expected = self.project_root / ".github" / "agents" / "my-agent" / "my-agent.agent.md" @@ -595,6 +605,27 @@ def test_nested_agent_bundle_filters_resources_and_preserves_agent_path(self): ".apm/agents/my-agent/guides/reference-doc.md, .apm/agents/my-agent/scripts/helper.py" ) + def test_prepare_agent_files_sanitizes_ignored_resource_diagnostic(self): + """Package-controlled diagnostic fields stay printable ASCII.""" + package_dir = self.project_root / "package" + agent_dir = package_dir / ".apm" / "agents" + agent_dir.mkdir(parents=True) + (agent_dir / "agent.agent.md").write_text("# Agent\n") + (agent_dir / "helper\nscript.py").write_text("print('helper')\n") + diagnostics = DiagnosticCollector() + + files = self.integrator.prepare_agent_files( + package_dir, + "unsafe\npackage", + diagnostics, + ) + + assert [path.name for path in files] == ["agent.agent.md"] + warnings = diagnostics.by_category()[CATEGORY_WARNING] + assert warnings[0].package == "unsafe?package" + assert warnings[0].detail == ".apm/agents/helper?script.py" + assert "Package required runtime resources as a skill bundle" in warnings[0].message + def test_get_target_filename_plain_md(self): """Plain .md files get renamed to .agent.md for .github/agents/.""" source = Path("/package/.apm/agents/context-architect.md") From 88d36719667029b631ac6e366e0d08cc74734962 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 30 Aug 2026 12:14:16 +0200 Subject: [PATCH 03/15] docs: clarify nested agent deployment Align the target matrix, warning guidance, package guide, and changelog with the nested-path contract and actionable resource warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + .../author-primitives/instructions-and-agents.md | 12 ++++++++---- docs/src/content/docs/reference/targets-matrix.md | 10 +++++----- .../.apm/skills/apm-usage/package-authoring.md | 5 +++-- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9911e9ce1..ebf279fdc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Agent discovery now preserves nested agent paths, validates plain Markdown definitions by `name` and `description` frontmatter, and warns when sibling resources cannot be deployed. (closes #2692) +- Agent discovery now preserves nested paths and warns about undeployable sibling resources. (#2721) - Git subdirectory dependencies with symlinks to files elsewhere in the same repository now install successfully where Git materializes symlinks; APM diff --git a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md index cffe78c0d7..c5deb247f4 100644 --- a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md +++ b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md @@ -143,9 +143,14 @@ Agent definitions in nested directories keep that relative directory in the target. Claude plugin manifests may also declare plain `.md` agent files; APM accepts those only when they contain non-empty `name` and `description` fields in YAML frontmatter. Other Markdown files and non-Markdown sibling resources -are not deployed as agents, and `apm install` lists them in a warning. If an -agent must ship scripts, templates, or other runtime resources, package it as a -skill bundle instead. +are not deployed as agents. `apm install` warns about them; use `--verbose` to +list the paths. If an agent must ship scripts, templates, or other runtime +resources, package it as a skill bundle instead. + +```text +.apm/agents/review/team-reviewer.agent.md + -> .github/agents/review/team-reviewer.agent.md +``` ### Frontmatter @@ -224,7 +229,6 @@ offending package and field so you can fix the source. | opencode | `.opencode/agents/.md` | verbatim | | codex | `.codex/agents/.toml` | `name` and `description` -> TOML; body becomes `developer_instructions`; unsupported `tools` emits a warning | | kiro | `.kiro/agents/.md` | `description`, `model`, `tools` kept; `name` and unknown fields stripped; identity from path; fail closed on unsupported tools (ref: [kiro.dev/docs/custom-agents](https://kiro.dev/docs/custom-agents/), accessed 2026-08-03) | -| grok-build | `.grok/agents/.md` | verbatim | | windsurf | not deployed | Windsurf has no agents primitive -- author personas as skills (Cascade auto-invokes by description) | | gemini | not deployed | Gemini CLI has no agents primitive | diff --git a/docs/src/content/docs/reference/targets-matrix.md b/docs/src/content/docs/reference/targets-matrix.md index 72037d4bc6..863a8fd4e3 100644 --- a/docs/src/content/docs/reference/targets-matrix.md +++ b/docs/src/content/docs/reference/targets-matrix.md @@ -115,7 +115,7 @@ GitHub Copilot (CLI and IDE). - **File conventions.** - instructions: `.github/instructions/.instructions.md` - prompts: `.github/prompts/.prompt.md` - - agents: `.github/agents/.agent.md` + - agents: `.github/agents/.agent.md` - skills: `.agents/skills//SKILL.md` at project scope and `~/.agents/skills//SKILL.md` at user scope - hooks: `.github/hooks/.json` @@ -139,7 +139,7 @@ Claude Code. - **File conventions.** - instructions: deployed directly by `apm install` to `.claude/rules/.md` - - agents: `.claude/agents/.md` + - agents: `.claude/agents/.md` - commands: `.claude/commands/.md` - skills: `.claude/skills//SKILL.md` - hooks: merged into `.claude/settings.json` @@ -155,7 +155,7 @@ Cursor. - **Supported primitives.** instructions, agents, skills, commands, hooks, mcp. (No `prompts`.) - **File conventions.** - instructions: `.cursor/rules/.mdc` - - agents: `.cursor/agents/.md` + - agents: `.cursor/agents/.md` - commands: `.cursor/commands/.md` - skills: `.agents/skills//SKILL.md` (project) or `~/.agents/skills//SKILL.md` (user) @@ -174,7 +174,7 @@ OpenAI Codex CLI. - **Deploy directory.** `.codex/` plus `.agents/` for skills. - **Supported primitives.** agents, skills, hooks, mcp. (No `instructions`, `prompts`, or `commands`.) - **File conventions.** - - agents: `.codex/agents/.toml` + - agents: `.codex/agents/.toml` - skills: `.agents/skills//SKILL.md` - hooks: `.codex/hooks.json` - **Compile output.** `AGENTS.md` only. Per-file instructions are not installed for Codex. @@ -214,7 +214,7 @@ OpenCode. - **Deploy directory.** `.opencode/` at project scope; `~/.config/opencode/` at user scope. - **Supported primitives.** agents, commands, skills, mcp. - **File conventions.** - - agents: `.opencode/agents/.md` + - agents: `.opencode/agents/.md` - commands: `.opencode/commands/.md` - skills: `.agents/skills//SKILL.md` (project) or `~/.config/opencode/skills//SKILL.md` (user) diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index a8d033c77f..0b68f1d451 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -344,8 +344,9 @@ Chat persona configuration. Place in `.apm/agents/`. Nested agent definitions preserve their relative directory when installed. Plain `.md` definitions from Claude plugins must declare non-empty `name` and `description` fields in YAML frontmatter; other Markdown and non-Markdown files -under the agent directory are skipped with an install warning. Use a skill -bundle when the runtime needs sibling scripts, templates, or other resources. +under the agent directory are skipped with an install warning. Run with +`--verbose` to list those paths. Use a skill bundle when the runtime needs +sibling scripts, templates, or other resources. ```yaml --- From c2b7328057ea6fe148308be4f6140edbc696c17f Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 30 Aug 2026 12:25:08 +0200 Subject: [PATCH 04/15] fix: satisfy install engine invariants Extract package-scoped primitive preparation and integration hints from the size-limited services module, and keep the existing diagnostic-owner mutation test targeted at its intended wrapper. Addresses CI failures from run 33306131435. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../checks/install_agent_inventory.py | 9 +++-- src/apm_cli/install/primitive_integration.py | 38 ++++++++++++++++++ src/apm_cli/install/services.py | 40 +------------------ src/apm_cli/integration/agent_integrator.py | 3 +- 4 files changed, 48 insertions(+), 42 deletions(-) create mode 100644 src/apm_cli/install/primitive_integration.py diff --git a/scripts/architecture_linter/checks/install_agent_inventory.py b/scripts/architecture_linter/checks/install_agent_inventory.py index fbffb70011..c77c20d7ca 100644 --- a/scripts/architecture_linter/checks/install_agent_inventory.py +++ b/scripts/architecture_linter/checks/install_agent_inventory.py @@ -13,6 +13,7 @@ _GUARD_AGENT_SOURCE_INVENTORY = "install-deployment-agent-source-inventory" _OWNER = "src/apm_cli/integration/agent_integrator.py" +_PREPARATION = "src/apm_cli/install/primitive_integration.py" _SERVICES = "src/apm_cli/install/services.py" _OWNER_DEFINITIONS = frozenset( {"_is_plain_md_agent", "_source_agent_relpath", "prepare_agent_files"} @@ -23,9 +24,10 @@ def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, .. """Agent admission, identity, and inventory must route through AgentIntegrator.""" rule_id = _GUARD_AGENT_SOURCE_INVENTORY owner, owner_fail = _facts_for(provider, _OWNER, rule_id) + preparation, preparation_fail = _facts_for(provider, _PREPARATION, rule_id) services, services_fail = _facts_for(provider, _SERVICES, rule_id) - if owner_fail or services_fail: - return tuple(list(owner_fail) + list(services_fail)) + if owner_fail or preparation_fail or services_fail: + return tuple(list(owner_fail) + list(preparation_fail) + list(services_fail)) definition_counts = dict.fromkeys(_OWNER_DEFINITIONS, 0) for path in _python_paths(provider, "src/apm_cli/"): @@ -44,7 +46,8 @@ def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, .. if ( any(count != 1 for count in definition_counts.values()) or any(not _present(owner, fragment) for fragment in required_owner_fragments) - or not _present(services, '"agent_files": integrator.prepare_agent_files(') + or not _present(preparation, '"agent_files": integrator.prepare_agent_files(') + or not _present(services, "prepare_primitive_inputs as _prepare_primitive_inputs") ): return ( _summary( diff --git a/src/apm_cli/install/primitive_integration.py b/src/apm_cli/install/primitive_integration.py new file mode 100644 index 0000000000..d995c1cc51 --- /dev/null +++ b/src/apm_cli/install/primitive_integration.py @@ -0,0 +1,38 @@ +"""Package-scoped preparation and presentation for primitive integration.""" + +from __future__ import annotations + +from typing import Any + + +def prepare_primitive_inputs( + primitive_name: str, + integrator: Any, + package_info: Any, + targets: Any, + diagnostics: Any, + source_plan: Any, +) -> dict[str, Any]: + """Prepare package-scoped inputs reused across target integrations.""" + if primitive_name != "agents" or not any( + target.primitives.get("agents") is not None for target in targets + ): + return {} + return { + "agent_files": integrator.prepare_agent_files( + package_info.install_path, + package_info.package.name, + diagnostics, + source_plan, + ) + } + + +def emit_integration_hints(primitive_name: str, info: dict, log_integration) -> None: + """Emit user actions that follow successful primitive integration.""" + if any(path.startswith("copilot-app/") for path in info["paths"]) and info["files"] > 0: + log_integration( + " |-- workflows arrive disabled; enable from the Copilot App's Workflows tab" + ) + if primitive_name == "canvas" and (info["files"] > 0 or info["adopted"] > 0): + log_integration(" |-- reload the Copilot session (/clear) or restart to load the canvas") diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index bd525f6ad6..c3ce000c6c 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -35,6 +35,8 @@ from .local_bundle_paths import bundle_slug_validation_error as _bundle_slug_error from .local_bundle_paths import known_bundle_deploy_prefixes as _known_bundle_prefixes from .local_bundle_paths import target_bundle_deploy_prefixes as _target_bundle_prefixes +from .primitive_integration import emit_integration_hints as _emit_integration_hints +from .primitive_integration import prepare_primitive_inputs as _prepare_primitive_inputs from .target_filter import resolve_effective_package_targets if TYPE_CHECKING: @@ -115,21 +117,6 @@ def _label_and_deploy_dir(prim_name: str, mapping, target, deploy_dir: str) -> t return prim_name, deploy_dir -def _emit_integration_hints(prim_name: str, info: dict, log_integration) -> None: - """Emit per-primitive 'next step' hints after an integration line.""" - # copilot-app workflows arrive disabled: the row lands enabled=0 and the - # user must flip the toggle in the Copilot App's Workflows tab before the - # schedule fires. - if any(p.startswith("copilot-app/") for p in info["paths"]) and info["files"] > 0: - log_integration( - " |-- workflows arrive disabled; enable from the Copilot App's Workflows tab" - ) - # Canvas extensions are discovered by Copilot CLI at session start, so a - # freshly-deployed canvas is not picked up mid-session. - if prim_name == "canvas" and (info["files"] > 0 or info["adopted"] > 0): - log_integration(" |-- reload the Copilot session (/clear) or restart to load the canvas") - - def _log_hooks_skip( package_name: str, package_info: Any, targets: Any, logger: InstallLogger | None ) -> None: @@ -214,29 +201,6 @@ def _log_package_target_restriction(logger: InstallLogger | None, target_selecti ) -def _prepare_primitive_inputs( - primitive_name: str, - integrator: Any, - package_info: Any, - targets: Any, - diagnostics: Any, - source_plan: Any, -) -> dict[str, Any]: - """Prepare package-scoped inputs reused across target integrations.""" - if primitive_name != "agents" or not any( - target.primitives.get("agents") is not None for target in targets - ): - return {} - return { - "agent_files": integrator.prepare_agent_files( - package_info.install_path, - package_info.package.name, - diagnostics, - source_plan, - ) - } - - def integrate_package_primitives( # noqa: PLR0913 package_info: Any, project_root: Path, diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index a419e7aedd..8cf4cc6215 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -133,9 +133,10 @@ def _warn_ignored_agent_resources( ) detail = ", ".join(relative) if diagnostics is not None: + safe_package = printable_ascii_text(package_name) diagnostics.warn( message=message, - package=printable_ascii_text(package_name), + package=safe_package, detail=detail, ) else: From 1d747ce439312c7e151cdd9e150f8b9265477971 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 30 Aug 2026 12:31:29 +0200 Subject: [PATCH 05/15] chore: sync generated APM state Refresh the architecture instruction hash and remove the stale generated agent so the repository self-audit matches the current local primitive inventory. apm-spec-waiver: Restores existing agent bundle deployment semantics without adding a new manifest contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/agents/algorithmic-patterns.agent.md | 152 ------------------- apm.lock.yaml | 11 -- 2 files changed, 163 deletions(-) delete mode 100644 .github/agents/algorithmic-patterns.agent.md diff --git a/.github/agents/algorithmic-patterns.agent.md b/.github/agents/algorithmic-patterns.agent.md deleted file mode 100644 index bd4e4afac8..0000000000 --- a/.github/agents/algorithmic-patterns.agent.md +++ /dev/null @@ -1,152 +0,0 @@ -# Algorithmic Performance Patterns - -Load this reference when the PR diff touches code outside the -transport/cache layer -- i.e. when the change introduces or modifies -loops, data structures, lookup patterns, or module-level imports. - -## Big O Quick Reference - -| Pattern | Complexity | Red Flag | -|---------|-----------|----------| -| Dict/set lookup | O(1) | Fine | -| List `.append` | O(1) amortised | Fine | -| `x in list` | O(n) | Use a set if called in a loop | -| Nested loops over same collection | O(n^2) | Extract an index dict first | -| Sort inside a loop | O(n^2 log n) | Sort once outside the loop | -| `any(pred(x) for x in coll)` in a loop | O(n*m) | Build a set/dict pre-loop | -| Unconditional full-dir scan on every write | O(n) per write = O(n^2) total | Track running total; scan only when needed | -| Linear search for identity match | O(n) per lookup | Build `{identity: index}` once | - -## Anti-Patterns to Flag - -### 1. Missing Index on Repeated Lookup - -```python -# BAD: O(n) per call, called m times = O(n*m) -def has_item(collection, key): - return any(item.key == key for item in collection) - -# GOOD: O(1) per call after O(n) setup -_index = {item.key for item in collection} -def has_item(key): - return key in _index -``` - -Flag when: a function does linear scan AND is called from within a -loop or from a method called repeatedly during resolution/install. - -### 2. Unconditional Expensive Operation - -```python -# BAD: scans entire cache dir on every store() -def store(self, url, body): - self._write(url, body) - self._enforce_size_cap() # full scandir every time - -# GOOD: fast-path skip when clearly under budget -def store(self, url, body): - self._write(url, body) - self._tracked_size += len(body) - if self._tracked_size > MAX_SIZE: - self._enforce_size_cap() # scan only when needed -``` - -Flag when: an expensive operation (directory walk, sort, full -re-computation) runs unconditionally on every call to a high-frequency -method. - -### 3. Triple-Pass Where Single-Pass Suffices - -```python -# BAD: iterates refs 3x (once per category) -for ref in refs: - if ref.startswith("refs/tags/"): ... -for ref in refs: - if ref.name == target: ... -for ref in refs: - if ref.name == f"refs/heads/{target}": ... - -# GOOD: single pass builds lookup dicts -tags, branches, by_name = {}, {}, {} -for ref in refs: - by_name[ref.name] = ref - if ref.name.startswith("refs/tags/"): - tags[strip_prefix(ref.name)] = ref - elif ref.name.startswith("refs/heads/"): - branches[ref.name[len("refs/heads/"):]] = ref -# Then O(1) lookups -``` - -Flag when: the same collection is iterated multiple times with -different predicates that could all be evaluated in one pass. - -### 4. Repeated Environment/Config Parsing - -```python -# BAD: re-parses on every call -def classify_host(hostname): - ghes = os.environ.get("GITHUB_HOST", "").strip().lower().split("/")[0] - # ... repeated in 5 other functions - -# GOOD: parse once in a helper -def _get_ghes_host(): - return os.environ.get("GITHUB_HOST", "").strip().lower().split("/")[0] -``` - -Flag when: the same `os.environ.get()` + normalisation chain appears -in multiple functions that are called in tight succession. - -### 5. Heavy Top-Level Imports on CLI Startup - -```python -# BAD: imports entire install engine at module level -from apm_cli.install.pipeline import FullPipeline # 40+ transitive modules - -# GOOD: defer to function scope -def install_command(): - from apm_cli.install.pipeline import FullPipeline - ... -``` - -Flag when: a command module imports heavy subpackages at the top level -that are not needed for other commands sharing the same CLI entrypoint. - -### 6. Synchronous Blocking in Parallelisable Paths - -```python -# BAD: sequential I/O in a loop -for pkg in packages: - metadata = fetch_metadata(pkg) # blocking network call - -# GOOD: parallel with bounded concurrency -with ThreadPoolExecutor(max_workers=8) as pool: - metadata_list = list(pool.map(fetch_metadata, packages)) -``` - -Flag when: a loop performs independent I/O operations (file reads, -network calls, subprocess spawns) that have no data dependency between -iterations. - -## Scaling Guard Pattern - -When reviewing a fix, recommend a scaling-guard test: -- Run the operation at size N and at size 10*N -- Assert the time ratio stays below a threshold (e.g. < 15x) -- This catches O(n^2) regressions without brittle absolute-time assertions - -```python -def test_scaling_ratio(): - t_small = median_time(lambda: operation(n=50)) - t_large = median_time(lambda: operation(n=500)) - ratio = t_large / t_small - assert ratio < 15, f"Ratio {ratio:.1f}x suggests O(n^2)" -``` - -## Quantification Checklist - -When reporting a performance finding, always state: -1. **What** -- the specific pattern (name it from the table above) -2. **Where** -- file:line range -3. **Frequency** -- how often this code path executes per typical run -4. **Complexity** -- the current Big O and the proposed Big O -5. **Fix** -- concrete code sketch (not just "optimise this") diff --git a/apm.lock.yaml b/apm.lock.yaml index 1c4699878d..27ebf673d0 100644 --- a/apm.lock.yaml +++ b/apm.lock.yaml @@ -2595,15 +2595,6 @@ deployments: - . active_owner: . content_hash: sha256:d1ea2d038e2af8be11d6c95b3213b03b9777fae46f0438efa95d5a803e6c3765 -- kind: project-relative - target: copilot - value: .github/agents/algorithmic-patterns.agent.md - runtime: null - scope: project - owners: - - . - active_owner: . - content_hash: sha256:278321484288eb8965a42c20cfa65ff055d250579849b1c48cbd3f553883fe5d - kind: project-relative target: copilot value: .github/agents/apm-ceo.agent.md @@ -3020,7 +3011,6 @@ local_deployed_files: - .agents/skills/supply-chain-security - .agents/skills/supply-chain-security/SKILL.md - .github/agents/agentic-workflows.agent.md -- .github/agents/algorithmic-patterns.agent.md - .github/agents/apm-ceo.agent.md - .github/agents/apm-primitives-architect.agent.md - .github/agents/auth-expert.agent.md @@ -3270,7 +3260,6 @@ local_deployed_file_hashes: .agents/skills/python-architecture/SKILL.md: sha256:f06e50b8c40d7e5a732a5405a603efd80da58de2b6f6b293f0f6b58f28eefe7b .agents/skills/supply-chain-security/SKILL.md: sha256:55ef10cd0f6b68e0db5a435a79851c055eab74d93fd552c6626df631deb51df4 .github/agents/agentic-workflows.agent.md: sha256:d1ea2d038e2af8be11d6c95b3213b03b9777fae46f0438efa95d5a803e6c3765 - .github/agents/algorithmic-patterns.agent.md: sha256:278321484288eb8965a42c20cfa65ff055d250579849b1c48cbd3f553883fe5d .github/agents/apm-ceo.agent.md: sha256:484da64428ea46a6183dffd3f30c9fc5fc5c747639c0c79e55be69dba0899323 .github/agents/apm-primitives-architect.agent.md: sha256:0c8f04297b14e144f84211b5b881b099233a9847c0fca257b9aa69c4d43d1eef .github/agents/auth-expert.agent.md: sha256:18264a933cba432b77d133e6ae11eee294c92ed245629af8c9b7a5bb7a9a300c From cbb93d90dff21c557b61de171b4c77336aacebc0 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 30 Aug 2026 13:00:01 +0200 Subject: [PATCH 06/15] fix: align nested agent consumers Route security scan admission through AgentIntegrator, preserve verbose uninstall diagnostics, and synchronize the remaining target and performance-reference documentation. Addresses the final review panel follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/agents/workflows/perf-scan.md | 4 ++-- .github/workflows/perf-scan.md | 6 ++--- .../content/docs/reference/targets-matrix.md | 2 +- src/apm_cli/commands/uninstall/engine.py | 2 +- src/apm_cli/install/deployable_source_plan.py | 6 +++-- .../unit/install/test_security_scan_scope.py | 24 +++++++++++++++++++ .../unit/test_surviving_deps_reintegration.py | 5 +++- 7 files changed, 39 insertions(+), 10 deletions(-) diff --git a/.github/agents/workflows/perf-scan.md b/.github/agents/workflows/perf-scan.md index 2c19b81ab2..5ab5caa8cc 100644 --- a/.github/agents/workflows/perf-scan.md +++ b/.github/agents/workflows/perf-scan.md @@ -17,7 +17,7 @@ a GitHub Issue on every run so the team has a daily record. The agent operates as the `performance-expert` persona defined in `.github/agents/performance-expert.agent.md`. When scanning non-transport code (i.e., anything outside `src/apm_cli/transport/`), it also loads the -pattern catalogue from `.github/agents/algorithmic-patterns.agent.md`. +pattern catalogue from `.apm/agents/references/algorithmic-patterns.md`. ## Anti-patterns checked @@ -67,5 +67,5 @@ This ensures the daily run is always visible in the issue tracker. - Workflow definition: `.github/workflows/perf-scan.md` - Performance agent: `.github/agents/performance-expert.agent.md` -- Pattern catalogue: `.github/agents/algorithmic-patterns.agent.md` +- Pattern catalogue: `.apm/agents/references/algorithmic-patterns.md` - Benchmark examples: `tests/benchmarks/test_perf_benchmarks.py` diff --git a/.github/workflows/perf-scan.md b/.github/workflows/perf-scan.md index adc3c164ea..7cf1ecf08b 100644 --- a/.github/workflows/perf-scan.md +++ b/.github/workflows/perf-scan.md @@ -39,7 +39,7 @@ actionable findings. Read `.github/agents/performance-expert.agent.md` for your full persona and mental model. When scanning non-transport code, also load -`.github/agents/algorithmic-patterns.agent.md` for the pattern catalogue. +`.apm/agents/references/algorithmic-patterns.md` for the pattern catalogue. ## Context @@ -59,8 +59,8 @@ Before scanning, read the algorithmic patterns reference so you know exactly wha to look for: ```bash -cat .github/agents/algorithmic-patterns.agent.md 2>/dev/null || \ - echo "[!] algorithmic-patterns.agent.md not found -- using inline patterns" +cat .apm/agents/references/algorithmic-patterns.md 2>/dev/null || \ + echo "[!] algorithmic-patterns.md not found -- using inline patterns" ``` Then read a sample of the source tree structure: diff --git a/docs/src/content/docs/reference/targets-matrix.md b/docs/src/content/docs/reference/targets-matrix.md index 863a8fd4e3..bb358bf54e 100644 --- a/docs/src/content/docs/reference/targets-matrix.md +++ b/docs/src/content/docs/reference/targets-matrix.md @@ -300,7 +300,7 @@ Cross-client shared skills directory. - **Deploy directory.** `.grok/` at project scope; `~/.grok/` at user scope. - **Supported primitives.** instructions, agents, commands, and skills. - **File conventions.** `.grok/rules/*.md`, - `.grok/agents/*.md`, `.grok/commands/*.md`, and + `.grok/agents/.md`, `.grok/commands/*.md`, and `.grok/skills//SKILL.md`. - **Compile behavior.** Produces `AGENTS.md`. diff --git a/src/apm_cli/commands/uninstall/engine.py b/src/apm_cli/commands/uninstall/engine.py index ef224165e6..e3b060c4cb 100644 --- a/src/apm_cli/commands/uninstall/engine.py +++ b/src/apm_cli/commands/uninstall/engine.py @@ -1295,7 +1295,7 @@ def _sync_integrations_after_uninstall( _rebuild_scope = InstallScope.USER if user_scope else InstallScope.PROJECT _allow_executables = getattr(apm_package, "allow_executables", None) - reintegration_diagnostics = DiagnosticCollector() + reintegration_diagnostics = DiagnosticCollector(verbose=logger.verbose) for dep_ref, pkg_info, authorized_targets in target_survivor_plan: dep_key = dep_ref.get_unique_key() deployed_files = package_deployed_files.setdefault(dep_key, []) diff --git a/src/apm_cli/install/deployable_source_plan.py b/src/apm_cli/install/deployable_source_plan.py index 1f2b04340e..73cd606fb4 100644 --- a/src/apm_cli/install/deployable_source_plan.py +++ b/src/apm_cli/install/deployable_source_plan.py @@ -118,8 +118,10 @@ def add_direct_matching_files(root: Path, pattern: str) -> None: add_matching_files(source_root / ".apm" / "prompts", "*.prompt.md") if "agents" in target_primitives: - add_direct_matching_files(source_root, "*.agent.md") - add_matching_files(source_root / ".apm" / "agents", "*.md") + from apm_cli.integration.agent_integrator import AgentIntegrator + + for path in AgentIntegrator().find_agent_files(source_root): + add_file(path) if "instructions" in target_primitives: add_matching_files(source_root / ".apm" / "instructions", "*.instructions.md") diff --git a/tests/unit/install/test_security_scan_scope.py b/tests/unit/install/test_security_scan_scope.py index f89c874665..2fd9f0c723 100644 --- a/tests/unit/install/test_security_scan_scope.py +++ b/tests/unit/install/test_security_scan_scope.py @@ -99,6 +99,30 @@ def test_source_only_hidden_character_is_not_in_authorized_scan(tmp_path: Path) assert _pre_deploy_security_scan(plan, DiagnosticCollector(), package_name="clean") is True +def test_non_agent_markdown_is_not_in_authorized_agent_scan(tmp_path: Path) -> None: + """Agent admission and pre-deploy scanning share one file vocabulary.""" + agents = tmp_path / ".apm" / "agents" + agents.mkdir(parents=True) + (agents / "reviewer.md").write_text( + "---\nname: reviewer\ndescription: Reviews changes\n---\n# Reviewer\n", + encoding="utf-8", + ) + (agents / "README.md").write_text("source-only \u202e documentation\n", encoding="utf-8") + + plan = DeployableSourcePlan.create( + _package(tmp_path), + [_primitive_target("agents")], + skill_subset=None, + hooks_approved=False, + canvas_approved=False, + skip_bin=True, + ) + verdict = SecurityGate.scan_files(tmp_path, path_filter=plan.includes) + + assert plan.paths == frozenset({".apm/agents/reviewer.md"}) + assert verdict.should_block is False + + def test_root_skill_plan_preserves_arbitrary_files_but_excludes_internal_content( tmp_path: Path, ) -> None: diff --git a/tests/unit/test_surviving_deps_reintegration.py b/tests/unit/test_surviving_deps_reintegration.py index 823e4ea0dd..4da9fa3eb3 100644 --- a/tests/unit/test_surviving_deps_reintegration.py +++ b/tests/unit/test_surviving_deps_reintegration.py @@ -358,6 +358,8 @@ def test_uninstall_reintegration_preserves_user_scope_and_denies_bin_trust( lockfile = LockFile() lockfile.add_dependency(LockedDependency(repo_url="acme/survivor", depth=1)) observed: list[dict] = [] + logger = MagicMock() + logger.verbose = True def _record_integration(*_args, **kwargs): observed.append(kwargs) @@ -371,7 +373,7 @@ def _record_integration(*_args, **kwargs): APMPackage.from_apm_yml(tmp_path / "apm.yml"), tmp_path, set(), - MagicMock(), + logger, lockfile=lockfile, user_scope=True, ) @@ -382,6 +384,7 @@ def _record_integration(*_args, **kwargs): assert observed[0]["scope"] is InstallScope.USER assert observed[0]["trust_bin"] is False assert observed[0]["bin_skip_reason_override"] == "not_retrusted_on_uninstall" + assert observed[0]["diagnostics"].verbose is True def test_hook_reintegration_sanitizes_blocked_dependency_identity( From 493fa0f7437b5b3f8981eaf744193a5541eb7341 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 30 Aug 2026 13:13:18 +0200 Subject: [PATCH 07/15] fix: unify Kiro agent identity routing Route Kiro through AgentIntegrator's canonical relative-path owner and extend AC36 so a target-specific identity helper cannot recreate split authority. Addresses the terminal Python Architect follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../checks/install_agent_inventory.py | 1 + src/apm_cli/integration/agent_integrator.py | 48 ++++--------------- 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/scripts/architecture_linter/checks/install_agent_inventory.py b/scripts/architecture_linter/checks/install_agent_inventory.py index c77c20d7ca..01cb5ef2d5 100644 --- a/scripts/architecture_linter/checks/install_agent_inventory.py +++ b/scripts/architecture_linter/checks/install_agent_inventory.py @@ -46,6 +46,7 @@ def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, .. if ( any(count != 1 for count in definition_counts.values()) or any(not _present(owner, fragment) for fragment in required_owner_fragments) + or _present(owner, "_kiro_agent_relpath") or not _present(preparation, '"agent_files": integrator.prepare_agent_files(') or not _present(services, "prepare_primitive_inputs as _prepare_primitive_inputs") ): diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index 8cf4cc6215..d2a913e0f6 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -127,8 +127,8 @@ def _warn_ignored_agent_resources( message = ( f"Ignored {len(relative)} non-agent file(s) under .apm/agents; " "only *.agent.md files and plain Markdown files with name and " - "description frontmatter are deployable. Run with --verbose to list " - "the files. Package required runtime resources as a skill bundle, " + "description frontmatter are deployable. Ignored file paths appear " + "in verbose output. Package required runtime resources as a skill bundle, " "then rerun 'apm install'." ) detail = ", ".join(relative) @@ -251,16 +251,12 @@ def integrate_agents_for_target( total_links_resolved = 0 for source_file in agent_files: - # kiro_agent uses relative path from .apm/agents/ for identity. - if mapping.format_id == "kiro_agent": - target_relpath = self._kiro_agent_relpath(source_file, package_info.install_path) - else: - target_relpath = self.get_target_filename_for_target( - source_file, - package_info.package.name, - target, - package_info.install_path, - ) + target_relpath = self.get_target_filename_for_target( + source_file, + package_info.package.name, + target, + package_info.install_path, + ) target_path = agents_dir / target_relpath # Defense-in-depth: assert containment under agents_dir so a # regression cannot smuggle a traversal sequence past the adopt @@ -589,34 +585,6 @@ def _write_codex_agent( # Kiro agent transformer (MD -> filtered MD) # ------------------------------------------------------------------ - @staticmethod - def _kiro_agent_relpath(source_file: Path, package_path: Path) -> str: - """Compute the relative target path for a Kiro agent file. - - Preserves subdirectory structure from .apm/agents/ so identity - derives from the deployed path, not from a 'name' frontmatter - field (Kiro CLI v3 / IDE uses relative path as identity). - - Sources under .apm/agents/ keep their relative subpath; root-level - sources are flattened to the filename only. - - Ref: https://kiro.dev/docs/custom-agents/ (accessed 2026-08-03) - """ - apm_agents_root = package_path / ".apm" / "agents" - try: - rel = source_file.relative_to(apm_agents_root) - except ValueError: - rel = Path(source_file.name) - parts = rel.parts - stem = parts[-1] - if stem.endswith(".agent.md"): - stem = stem[: -len(".agent.md")] + ".md" - elif not stem.endswith(".md"): - stem = stem + ".md" - if len(parts) > 1: - return str(Path(*parts[:-1]) / stem) - return stem - @staticmethod def _preflight_render_kiro_agent( source: Path, From 8d628fb75596c7f0beade22cde5ae877ed48f96f Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Mon, 31 Aug 2026 09:05:22 +0200 Subject: [PATCH 08/15] fix: fold final agent review safeguards Route plain Markdown admission through the canonical frontmatter loader, bound sibling diagnostics, and prove the real multi-target service path. Addresses CEO follow-ups from the PR #2721 review panel. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 6 +- .../checks/install_agent_inventory.py | 1 + src/apm_cli/integration/agent_integrator.py | 35 +++++------ .../test_marketplace_plugin_integration.py | 63 ++++++++++++------- .../unit/integration/test_agent_integrator.py | 43 ++++++++++++- 5 files changed, 102 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebf279fdc1..dbad3ffeb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,10 +47,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm doctor` now reports malformed project executable-trust configuration under either `executables` or the deprecated `allowExecutables` key as an actionable informational warning instead of omitting the check. (#2719) -- Agent discovery now preserves nested agent paths, validates plain Markdown - definitions by `name` and `description` frontmatter, and warns when sibling - resources cannot be deployed. (closes #2692) -- Agent discovery now preserves nested paths and warns about undeployable sibling resources. (#2721) +- Agent discovery now preserves nested paths, validates plain Markdown agent + frontmatter, and warns about undeployable sibling resources. (#2721) - Git subdirectory dependencies with symlinks to files elsewhere in the same repository now install successfully where Git materializes symlinks; APM diff --git a/scripts/architecture_linter/checks/install_agent_inventory.py b/scripts/architecture_linter/checks/install_agent_inventory.py index 01cb5ef2d5..d8516958c5 100644 --- a/scripts/architecture_linter/checks/install_agent_inventory.py +++ b/scripts/architecture_linter/checks/install_agent_inventory.py @@ -41,6 +41,7 @@ def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, .. required_owner_fragments = ( "files, _ignored = self._classify_agent_files(package_path)", "agent_files, ignored_resources = self._classify_agent_files(package_path)", + "frontmatter = load_frontmatter(str(source)).metadata", "if agent_files is None:", ) if ( diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index d2a913e0f6..251ff7b178 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -20,7 +20,7 @@ from apm_cli.utils.diagnostics import printable_ascii_text from apm_cli.utils.path_security import PathTraversalError, ensure_path_within from apm_cli.utils.paths import portable_relpath -from apm_cli.utils.yaml_io import load_yaml_str, yaml_to_str +from apm_cli.utils.yaml_io import load_frontmatter, load_yaml_str, yaml_to_str if TYPE_CHECKING: from apm_cli.integration.targets import TargetProfile @@ -44,6 +44,7 @@ "*", } ) +_IGNORED_AGENT_RESOURCE_DETAIL_LIMIT = 20 class AgentIntegrator(BaseIntegrator): @@ -90,15 +91,8 @@ def _classify_agent_files(self, package_path: Path) -> tuple[list[Path], list[Pa def _is_plain_md_agent(source: Path) -> bool: """Return whether a plain Markdown file declares agent frontmatter.""" try: - content = source.read_text(encoding="utf-8") - except (OSError, UnicodeError): - return False - match = AgentIntegrator._FRONTMATTER_RE.match(content) - if match is None: - return False - try: - frontmatter = load_yaml_str(match.group(1)) - except yaml.YAMLError: + frontmatter = load_frontmatter(str(source)).metadata + except (OSError, UnicodeError, yaml.YAMLError): return False if not isinstance(frontmatter, dict): return False @@ -122,25 +116,30 @@ def _warn_ignored_agent_resources( if not ignored_resources: return relative = sorted( - printable_ascii_text(portable_relpath(path, package_path)) for path in ignored_resources + printable_ascii_text(path.relative_to(package_path).as_posix()) + for path in ignored_resources ) + noun = "file" if len(relative) == 1 else "files" message = ( - f"Ignored {len(relative)} non-agent file(s) under .apm/agents; " + f"Ignored {len(relative)} non-agent {noun} under .apm/agents; " "only *.agent.md files and plain Markdown files with name and " - "description frontmatter are deployable. Ignored file paths appear " - "in verbose output. Package required runtime resources as a skill bundle, " - "then rerun 'apm install'." + "description frontmatter are deployable. Package required runtime " + "resources as a skill bundle, then rerun 'apm install'." ) - detail = ", ".join(relative) if diagnostics is not None: safe_package = printable_ascii_text(package_name) + visible = relative[:_IGNORED_AGENT_RESOURCE_DETAIL_LIMIT] + omitted = len(relative) - len(visible) + detail = ", ".join(visible) + if omitted: + detail = f"{detail}, ... (+{omitted} more)" diagnostics.warn( - message=message, + message=f"{message} Ignored file paths appear in verbose output.", package=safe_package, detail=detail, ) else: - _rich_warning(f"{message} Files: {detail}") + _rich_warning(message) def prepare_agent_files( self, diff --git a/tests/integration/test_marketplace_plugin_integration.py b/tests/integration/test_marketplace_plugin_integration.py index 957fdc577b..f6a210313f 100644 --- a/tests/integration/test_marketplace_plugin_integration.py +++ b/tests/integration/test_marketplace_plugin_integration.py @@ -9,15 +9,18 @@ import json import shutil +from dataclasses import replace from datetime import datetime from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock, patch import pytest from click.testing import CliRunner from apm_cli.commands.install import install from apm_cli.deps.plugin_parser import _map_plugin_artifacts -from apm_cli.install.deployable_source_plan import DeployableSourcePlan +from apm_cli.install.services import IntegratorBundle, integrate_package_primitives from apm_cli.integration.agent_integrator import AgentIntegrator from apm_cli.integration.command_integrator import CommandIntegrator from apm_cli.integration.prompt_integrator import PromptIntegrator @@ -269,40 +272,54 @@ def test_nested_agent_bundle_maps_and_deploys_to_multiple_targets(self, tmp_path ), installed_at=datetime.now().isoformat(), ) - targets = [KNOWN_TARGETS["copilot"], KNOWN_TARGETS["claude"]] + targets = [ + replace( + KNOWN_TARGETS[target_name], + primitives={"agents": KNOWN_TARGETS[target_name].primitives["agents"]}, + ) + for target_name in ("copilot", "claude") + ] (tmp_path / ".claude").mkdir() - source_plan = DeployableSourcePlan.create( - package_info, - targets, - skill_subset=None, - hooks_approved=False, - canvas_approved=False, - skip_bin=True, - ) diagnostics = DiagnosticCollector() integrator = AgentIntegrator() - agent_files = integrator.prepare_agent_files( - plugin_dir, - package.name, - diagnostics, - source_plan, + hook_integrator = MagicMock() + hook_integrator.reconcile_package_target_restriction = None + skill_integrator = MagicMock() + skill_integrator.integrate_package_skill.return_value = SimpleNamespace( + target_paths=[], + skill_created=False, + sub_skills_promoted=0, + bin_deployed=0, + bin_skipped_reason=None, ) - results = [ - integrator.integrate_agents_for_target( - target, + with patch.object( + integrator, + "prepare_agent_files", + wraps=integrator.prepare_agent_files, + ) as prepare_agent_files: + result = integrate_package_primitives( package_info, tmp_path, + targets=targets, + integrators=IntegratorBundle( + prompt=MagicMock(), + agent=integrator, + skill=skill_integrator, + instruction=MagicMock(), + command=MagicMock(), + hook=hook_integrator, + ), + force=False, + managed_files=set(), diagnostics=diagnostics, - source_plan=source_plan, - agent_files=agent_files, + package_name=package.name, ) - for target in targets - ] + prepare_agent_files.assert_called_once() assert (tmp_path / ".github/agents/my-agent/my-agent.agent.md").is_file() assert (tmp_path / ".claude/agents/my-agent/my-agent.md").is_file() - assert sum(result.files_integrated for result in results) == 2 + assert result["agents"] == 2 warnings = diagnostics.by_category()[CATEGORY_WARNING] assert len(warnings) == 1 assert warnings[0].detail == ".apm/agents/my-agent/scripts/helper.py" diff --git a/tests/unit/integration/test_agent_integrator.py b/tests/unit/integration/test_agent_integrator.py index de4beafe1c..113ed54591 100644 --- a/tests/unit/integration/test_agent_integrator.py +++ b/tests/unit/integration/test_agent_integrator.py @@ -3,7 +3,7 @@ import tempfile from datetime import datetime from pathlib import Path -from unittest.mock import Mock +from unittest.mock import Mock, patch from apm_cli.install.deployable_source_plan import DeployableSourcePlan from apm_cli.integration import AgentIntegrator @@ -531,6 +531,18 @@ def test_find_agent_files_requires_frontmatter_for_plain_md(self): assert names == {"planner.md", "coder.md"} + def test_find_agent_files_accepts_bom_prefixed_plain_agent(self): + """BOM-prefixed Markdown routes through canonical frontmatter parsing.""" + package_dir = self.project_root / "package" + apm_agents = package_dir / ".apm" / "agents" + apm_agents.mkdir(parents=True) + agent = apm_agents / "planner.md" + agent.write_bytes( + b"\xef\xbb\xbf---\nname: planner\ndescription: Plans implementation work\n---\n# Planner\n" + ) + + assert self.integrator.find_agent_files(package_dir) == [agent] + def test_find_agent_files_discovers_nested_subdirectories(self): """find_agent_files uses rglob so agents in subdirs are found.""" package_dir = self.project_root / "package" @@ -626,6 +638,35 @@ def test_prepare_agent_files_sanitizes_ignored_resource_diagnostic(self): assert warnings[0].detail == ".apm/agents/helper?script.py" assert "Package required runtime resources as a skill bundle" in warnings[0].message + def test_prepare_agent_files_caps_ignored_resource_detail(self): + """Ignored-resource diagnostics stay bounded for large bundles.""" + package_dir = self.project_root / "package" + agent_dir = package_dir / ".apm" / "agents" + agent_dir.mkdir(parents=True) + for index in range(25): + (agent_dir / f"resource-{index:02}.txt").write_text("resource\n") + diagnostics = DiagnosticCollector() + + self.integrator.prepare_agent_files(package_dir, "large-package", diagnostics) + + warning = diagnostics.by_category()[CATEGORY_WARNING][0] + assert warning.detail.count(".apm/agents/resource-") == 20 + assert warning.detail.endswith(", ... (+5 more)") + + def test_prepare_agent_files_fallback_keeps_paths_out_of_summary(self): + """Fallback warnings stay concise when no verbose collector exists.""" + package_dir = self.project_root / "package" + agent_dir = package_dir / ".apm" / "agents" + agent_dir.mkdir(parents=True) + (agent_dir / "helper.py").write_text("print('helper')\n") + + with patch("apm_cli.integration.agent_integrator._rich_warning") as warning: + self.integrator.prepare_agent_files(package_dir, "test-package") + + message = warning.call_args.args[0] + assert "helper.py" not in message + assert "verbose output" not in message + def test_get_target_filename_plain_md(self): """Plain .md files get renamed to .agent.md for .github/agents/.""" source = Path("/package/.apm/agents/context-architect.md") From da8b362e8970b3fd8ef63f5fd212954a36205eef Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Mon, 31 Aug 2026 09:21:50 +0200 Subject: [PATCH 09/15] docs: clarify portable plain agent support Describe plain Markdown admission as an .apm/agents compatibility contract rather than a Claude-plugin-only behavior. Addresses the final Doc Writer follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../author-primitives/instructions-and-agents.md | 12 ++++++------ .../.apm/skills/apm-usage/package-authoring.md | 7 ++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md index c5deb247f4..f05230004d 100644 --- a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md +++ b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md @@ -140,12 +140,12 @@ my-package/ File names end in `.agent.md` and live under `.apm/agents/`. Agent definitions in nested directories keep that relative directory in the -target. Claude plugin manifests may also declare plain `.md` agent files; APM -accepts those only when they contain non-empty `name` and `description` fields -in YAML frontmatter. Other Markdown files and non-Markdown sibling resources -are not deployed as agents. `apm install` warns about them; use `--verbose` to -list the paths. If an agent must ship scripts, templates, or other runtime -resources, package it as a skill bundle instead. +target. For compatibility, `.apm/agents/` may also contain plain `.md` agent +files; APM accepts those only when they contain non-empty `name` and +`description` fields in YAML frontmatter. Other Markdown files and non-Markdown +sibling resources are not deployed as agents. `apm install` warns about them; +use `--verbose` to list the paths. If an agent must ship scripts, templates, or +other runtime resources, package it as a skill bundle instead. ```text .apm/agents/review/team-reviewer.agent.md diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 0b68f1d451..3e75a07a2b 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -342,9 +342,10 @@ supported top-level harness directories: `.agents`, `.apm`, `.claude`, Chat persona configuration. Place in `.apm/agents/`. Nested agent definitions preserve their relative directory when installed. -Plain `.md` definitions from Claude plugins must declare non-empty `name` and -`description` fields in YAML frontmatter; other Markdown and non-Markdown files -under the agent directory are skipped with an install warning. Run with +For compatibility, plain `.md` definitions under `.apm/agents/` must declare +non-empty `name` and `description` fields in YAML frontmatter; other Markdown +and non-Markdown files under the agent directory are skipped with an install +warning. Run with `--verbose` to list those paths. Use a skill bundle when the runtime needs sibling scripts, templates, or other resources. From 2ba271c7233aa332577a3bd13b4a546a6a44333e Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 1 Sep 2026 06:17:48 +0200 Subject: [PATCH 10/15] test: register agent inventory architecture rule Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/unit/scripts/test_architecture_runner.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index a8fba7d660..b7818bc09b 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -606,6 +606,7 @@ def exiting_import( contracts-tooling-frontmatter-yaml contracts-tooling-generation-footer install-deployment-approval-outcome-routing +install-deployment-agent-source-inventory install-deployment-audit-policy-discovery install-deployment-audit-replay install-deployment-base-integrator From cf1ac1adf747a38f06b60b667e48d86c0cf7f10c Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 1 Sep 2026 09:06:44 +0200 Subject: [PATCH 11/15] fix: keep legacy agent projection flat Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../owners/install-deployment.json | 4 +- CHANGELOG.md | 5 +- .../instructions-and-agents.md | 36 +++++----- .../content/docs/reference/targets-matrix.md | 16 +++-- .../skills/apm-usage/package-authoring.md | 17 +++-- .../checks/install_agent_inventory.py | 13 ++-- .../checks/install_deployment_analyzers.py | 2 +- src/apm_cli/deps/plugin_parser.py | 17 ++--- src/apm_cli/integration/agent_integrator.py | 72 ++++++++++--------- .../test_marketplace_plugin_integration.py | 10 +-- .../unit/integration/test_agent_integrator.py | 6 +- tests/unit/test_plugin_parser.py | 7 +- 12 files changed, 108 insertions(+), 97 deletions(-) diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index 25528c83fa..d8e1b6c326 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -52,8 +52,8 @@ }, { "id": "agent-source-admission-inventory", - "decision": "Agent source admission, relative identity, and package-level inventory", - "owner": "integration/agent_integrator.py (prepare_agent_files, _is_plain_md_agent, _source_agent_relpath)", + "decision": "Agent source admission and package-level inventory", + "owner": "integration/agent_integrator.py (prepare_agent_files, _is_plain_md_agent)", "selectors": ["src/apm_cli/integration/agent_integrator.py"], "guards": ["install-deployment-agent-source-inventory"] }, diff --git a/CHANGELOG.md b/CHANGELOG.md index dbad3ffeb8..5631fe5c6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,8 +47,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm doctor` now reports malformed project executable-trust configuration under either `executables` or the deprecated `allowExecutables` key as an actionable informational warning instead of omitting the check. (#2719) -- Agent discovery now preserves nested paths, validates plain Markdown agent - frontmatter, and warns about undeployable sibling resources. (#2721) +- Legacy agent projection now stays flat for harness discovery, admits plain + Markdown only with non-empty `name` and `description` frontmatter, and emits + one actionable warning when it skips unsupported sibling resources. (#2721) - Git subdirectory dependencies with symlinks to files elsewhere in the same repository now install successfully where Git materializes symlinks; APM diff --git a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md index f05230004d..531b4936c5 100644 --- a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md +++ b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md @@ -139,18 +139,18 @@ my-package/ File names end in `.agent.md` and live under `.apm/agents/`. -Agent definitions in nested directories keep that relative directory in the -target. For compatibility, `.apm/agents/` may also contain plain `.md` agent -files; APM accepts those only when they contain non-empty `name` and -`description` fields in YAML frontmatter. Other Markdown files and non-Markdown -sibling resources are not deployed as agents. `apm install` warns about them; -use `--verbose` to list the paths. If an agent must ship scripts, templates, or -other runtime resources, package it as a skill bundle instead. - -```text -.apm/agents/review/team-reviewer.agent.md - -> .github/agents/review/team-reviewer.agent.md -``` +For flat-file harnesses, legacy integration flattens nested definitions into +the target's agents directory because those harnesses may not discover nested +agents. Plain `.md` files under `.apm/agents/` are agents only when YAML +frontmatter contains non-empty `name` and `description` fields. APM skips other +Markdown and non-Markdown sibling resources and emits one actionable warning; +use `--verbose` to list the paths. Package runtime resources in a skill bundle. + +:::note[Planned] +Agent Plugins 1.0 targets full bundle preservation. Today APM preserves bundles +natively only for Copilot, and only for skills and MCP servers. See +[Pack a bundle](../../pack-a-bundle/#what-apm-pack-produces). +::: ### Frontmatter @@ -222,12 +222,12 @@ offending package and field so you can fix the source. | Target | Output path | Transform | |---|---|---| -| copilot | `.github/agents/.agent.md` | verbatim | -| claude | `.claude/agents/.md` | verbatim | -| grok-build | `.grok/agents/.md` | verbatim | -| cursor | `.cursor/agents/.md` | verbatim | -| opencode | `.opencode/agents/.md` | verbatim | -| codex | `.codex/agents/.toml` | `name` and `description` -> TOML; body becomes `developer_instructions`; unsupported `tools` emits a warning | +| copilot | `.github/agents/.agent.md` | verbatim | +| claude | `.claude/agents/.md` | verbatim | +| grok-build | `.grok/agents/.md` | verbatim | +| cursor | `.cursor/agents/.md` | verbatim | +| opencode | `.opencode/agents/.md` | verbatim | +| codex | `.codex/agents/.toml` | `name` and `description` -> TOML; body becomes `developer_instructions`; unsupported `tools` emits a warning | | kiro | `.kiro/agents/.md` | `description`, `model`, `tools` kept; `name` and unknown fields stripped; identity from path; fail closed on unsupported tools (ref: [kiro.dev/docs/custom-agents](https://kiro.dev/docs/custom-agents/), accessed 2026-08-03) | | windsurf | not deployed | Windsurf has no agents primitive -- author personas as skills (Cascade auto-invokes by description) | | gemini | not deployed | Gemini CLI has no agents primitive | diff --git a/docs/src/content/docs/reference/targets-matrix.md b/docs/src/content/docs/reference/targets-matrix.md index bb358bf54e..a78aa87d80 100644 --- a/docs/src/content/docs/reference/targets-matrix.md +++ b/docs/src/content/docs/reference/targets-matrix.md @@ -115,7 +115,7 @@ GitHub Copilot (CLI and IDE). - **File conventions.** - instructions: `.github/instructions/.instructions.md` - prompts: `.github/prompts/.prompt.md` - - agents: `.github/agents/.agent.md` + - agents: `.github/agents/.agent.md` - skills: `.agents/skills//SKILL.md` at project scope and `~/.agents/skills//SKILL.md` at user scope - hooks: `.github/hooks/.json` @@ -128,6 +128,10 @@ GitHub Copilot (CLI and IDE). CLI can invoke them from any working directory. - **Global compile.** `apm compile -g` can also render global instructions to `~/.copilot/AGENTS.md` for root-context readers that honor `AGENTS.md`. +:::note[Planned] +Agent Plugins 1.0 targets full bundle preservation. Today APM preserves bundles +natively only for Copilot, and only for skills and MCP servers. +::: ## claude @@ -139,7 +143,7 @@ Claude Code. - **File conventions.** - instructions: deployed directly by `apm install` to `.claude/rules/.md` - - agents: `.claude/agents/.md` + - agents: `.claude/agents/.md` - commands: `.claude/commands/.md` - skills: `.claude/skills//SKILL.md` - hooks: merged into `.claude/settings.json` @@ -155,7 +159,7 @@ Cursor. - **Supported primitives.** instructions, agents, skills, commands, hooks, mcp. (No `prompts`.) - **File conventions.** - instructions: `.cursor/rules/.mdc` - - agents: `.cursor/agents/.md` + - agents: `.cursor/agents/.md` - commands: `.cursor/commands/.md` - skills: `.agents/skills//SKILL.md` (project) or `~/.agents/skills//SKILL.md` (user) @@ -174,7 +178,7 @@ OpenAI Codex CLI. - **Deploy directory.** `.codex/` plus `.agents/` for skills. - **Supported primitives.** agents, skills, hooks, mcp. (No `instructions`, `prompts`, or `commands`.) - **File conventions.** - - agents: `.codex/agents/.toml` + - agents: `.codex/agents/.toml` - skills: `.agents/skills//SKILL.md` - hooks: `.codex/hooks.json` - **Compile output.** `AGENTS.md` only. Per-file instructions are not installed for Codex. @@ -214,7 +218,7 @@ OpenCode. - **Deploy directory.** `.opencode/` at project scope; `~/.config/opencode/` at user scope. - **Supported primitives.** agents, commands, skills, mcp. - **File conventions.** - - agents: `.opencode/agents/.md` + - agents: `.opencode/agents/.md` - commands: `.opencode/commands/.md` - skills: `.agents/skills//SKILL.md` (project) or `~/.config/opencode/skills//SKILL.md` (user) @@ -300,7 +304,7 @@ Cross-client shared skills directory. - **Deploy directory.** `.grok/` at project scope; `~/.grok/` at user scope. - **Supported primitives.** instructions, agents, commands, and skills. - **File conventions.** `.grok/rules/*.md`, - `.grok/agents/.md`, `.grok/commands/*.md`, and + `.grok/agents/*.md`, `.grok/commands/*.md`, and `.grok/skills//SKILL.md`. - **Compile behavior.** Produces `AGENTS.md`. diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 3e75a07a2b..33cd3390ee 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -341,13 +341,16 @@ supported top-level harness directories: `.agents`, `.apm`, `.claude`, Chat persona configuration. Place in `.apm/agents/`. -Nested agent definitions preserve their relative directory when installed. -For compatibility, plain `.md` definitions under `.apm/agents/` must declare -non-empty `name` and `description` fields in YAML frontmatter; other Markdown -and non-Markdown files under the agent directory are skipped with an install -warning. Run with -`--verbose` to list those paths. Use a skill bundle when the runtime needs -sibling scripts, templates, or other resources. +Legacy integration flattens nested agent definitions for flat-file harnesses +because they may not discover nested agents. Plain `.md` +files under `.apm/agents/` must declare non-empty `name` and `description` +fields in YAML frontmatter. Other Markdown and non-Markdown sibling resources +are skipped with one actionable warning; use `--verbose` to list the paths. +Package runtime resources in a skill bundle. + +Full bundle preservation is planned for Agent Plugins 1.0; it is not current +legacy projection behavior. Today APM preserves bundles natively only for +Copilot, and only for skills and MCP servers. ```yaml --- diff --git a/scripts/architecture_linter/checks/install_agent_inventory.py b/scripts/architecture_linter/checks/install_agent_inventory.py index d8516958c5..f92d813eb8 100644 --- a/scripts/architecture_linter/checks/install_agent_inventory.py +++ b/scripts/architecture_linter/checks/install_agent_inventory.py @@ -15,13 +15,11 @@ _OWNER = "src/apm_cli/integration/agent_integrator.py" _PREPARATION = "src/apm_cli/install/primitive_integration.py" _SERVICES = "src/apm_cli/install/services.py" -_OWNER_DEFINITIONS = frozenset( - {"_is_plain_md_agent", "_source_agent_relpath", "prepare_agent_files"} -) +_OWNER_DEFINITIONS = frozenset({"_is_plain_md_agent", "prepare_agent_files"}) def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, ...]: - """Agent admission, identity, and inventory must route through AgentIntegrator.""" + """Agent admission and inventory must route through AgentIntegrator.""" rule_id = _GUARD_AGENT_SOURCE_INVENTORY owner, owner_fail = _facts_for(provider, _OWNER, rule_id) preparation, preparation_fail = _facts_for(provider, _PREPARATION, rule_id) @@ -42,12 +40,15 @@ def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, .. "files, _ignored = self._classify_agent_files(package_path)", "agent_files, ignored_resources = self._classify_agent_files(package_path)", "frontmatter = load_frontmatter(str(source)).metadata", + 'name = frontmatter.get("name")', + 'description = frontmatter.get("description")', + "and bool(name.strip())", + "and bool(description.strip())", "if agent_files is None:", ) if ( any(count != 1 for count in definition_counts.values()) or any(not _present(owner, fragment) for fragment in required_owner_fragments) - or _present(owner, "_kiro_agent_relpath") or not _present(preparation, '"agent_files": integrator.prepare_agent_files(') or not _present(services, "prepare_primitive_inputs as _prepare_primitive_inputs") ): @@ -55,7 +56,7 @@ def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, .. _summary( rule_id, _OWNER, - "Agent admission, relative identity, and inventory must route through AgentIntegrator", + "Agent admission and inventory must route through AgentIntegrator", ), ) return () diff --git a/scripts/architecture_linter/checks/install_deployment_analyzers.py b/scripts/architecture_linter/checks/install_deployment_analyzers.py index 1237dff7d9..106620423d 100644 --- a/scripts/architecture_linter/checks/install_deployment_analyzers.py +++ b/scripts/architecture_linter/checks/install_deployment_analyzers.py @@ -60,7 +60,7 @@ RULES: tuple[Rule, ...] = ( _rule( _GUARD_AGENT_SOURCE_INVENTORY, - "Agent admission, relative identity, and inventory stay owned by AgentIntegrator.", + "Agent admission and inventory stay owned by AgentIntegrator.", check_agent_source_inventory, ), _rule( diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py index 8cd217ddbf..817292de3b 100644 --- a/src/apm_cli/deps/plugin_parser.py +++ b/src/apm_cli/deps/plugin_parser.py @@ -1148,28 +1148,21 @@ def _is_same_path(src: Path, dst: Path) -> bool: return False # Map agents/ - # The top-level agents/ directory is a collection, so merge its contents - # directly into .apm/agents/. A manifest may instead declare one of its - # child directories as an agent bundle. Preserve that relative directory - # so nested agent identity and sibling resources do not get flattened. + # Unlike skills (which are named directories containing SKILL.md), agents + # are flat files -- each admitted Markdown file is one agent. Always merge + # directory contents directly into .apm/agents/. agent_sources = _resolve_sources("agents", "agents") if agent_sources: target_agents = apm_dir / "agents" - default_agents = (plugin_path / "agents").resolve() _assert_no_symlink_descendants(target_agents) agent_dirs = [s for s in agent_sources if s.is_dir()] agent_files = [s for s in agent_sources if s.is_file()] for d in agent_dirs: - try: - relative_bundle = d.relative_to(default_agents) - except ValueError: - relative_bundle = Path() - destination = target_agents / relative_bundle - if _is_same_path(d, destination): + if _is_same_path(d, target_agents): continue shutil.copytree( d, - destination, + target_agents, dirs_exist_ok=True, ignore=ignore_non_content, ) diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index 251ff7b178..0f42b2e9a8 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -158,21 +158,6 @@ def prepare_agent_files( ) return self.filter_authorized_files(agent_files, source_plan) - @staticmethod - def _source_agent_relpath(source_file: Path, package_path: Path | None = None) -> Path: - """Return an agent's path relative to the canonical agents directory.""" - if package_path is not None: - try: - return source_file.relative_to(package_path / ".apm" / "agents") - except ValueError: - return Path(source_file.name) - - parts = source_file.parts - for index in range(len(parts) - 1): - if parts[index : index + 2] == (".apm", "agents"): - return Path(*parts[index + 2 :]) - return Path(source_file.name) - # NOTE: find_skill_file(), integrate_skill(), and _generate_skill_agent_content() # have been REMOVED as part of T5 (skill-strategy.md). # @@ -190,14 +175,12 @@ def get_target_filename_for_target( source_file: Path, package_name: str, target: TargetProfile, - package_path: Path | None = None, ) -> str: - """Generate a target-relative path using the target agent extension.""" + """Generate target filename using the extension from *target*'s agents mapping.""" mapping = target.primitives.get("agents") ext = mapping.extension if mapping else ".agent.md" stem = source_file.name[:-9] if source_file.name.endswith(".agent.md") else source_file.stem - source_relpath = self._source_agent_relpath(source_file, package_path) - return (source_relpath.parent / f"{stem}{ext}").as_posix() + return f"{stem}{ext}" def integrate_agents_for_target( self, @@ -250,12 +233,16 @@ def integrate_agents_for_target( total_links_resolved = 0 for source_file in agent_files: - target_relpath = self.get_target_filename_for_target( - source_file, - package_info.package.name, - target, - package_info.install_path, - ) + # Kiro uses the relative source path as agent identity. Other + # harnesses discover flat files directly under their agents root. + if mapping.format_id == "kiro_agent": + target_relpath = self._kiro_agent_relpath(source_file, package_info.install_path) + else: + target_relpath = self.get_target_filename_for_target( + source_file, + package_info.package.name, + target, + ) target_path = agents_dir / target_relpath # Defense-in-depth: assert containment under agents_dir so a # regression cannot smuggle a traversal sequence past the adopt @@ -333,7 +320,6 @@ def integrate_agents_for_target( files_skipped += 1 continue - target_path.parent.mkdir(parents=True, exist_ok=True) if mapping.format_id == "codex_agent": self._write_codex_agent( source_file, @@ -584,6 +570,34 @@ def _write_codex_agent( # Kiro agent transformer (MD -> filtered MD) # ------------------------------------------------------------------ + @staticmethod + def _kiro_agent_relpath(source_file: Path, package_path: Path) -> str: + """Compute the relative target path for a Kiro agent file. + + Preserves subdirectory structure from .apm/agents/ so identity + derives from the deployed path, not from a 'name' frontmatter + field (Kiro CLI v3 / IDE uses relative path as identity). + + Sources under .apm/agents/ keep their relative subpath; root-level + sources are flattened to the filename only. + + Ref: https://kiro.dev/docs/custom-agents/ (accessed 2026-08-03) + """ + apm_agents_root = package_path / ".apm" / "agents" + try: + rel = source_file.relative_to(apm_agents_root) + except ValueError: + rel = Path(source_file.name) + parts = rel.parts + stem = parts[-1] + if stem.endswith(".agent.md"): + stem = stem[: -len(".agent.md")] + ".md" + elif not stem.endswith(".md"): + stem = stem + ".md" + if len(parts) > 1: + return str(Path(*parts[:-1]) / stem) + return stem + @staticmethod def _preflight_render_kiro_agent( source: Path, @@ -777,7 +791,6 @@ def integrate_package_agents( source_file, package_info.package.name, copilot, - package_info.install_path, ) target_path = agents_dir / target_filename try: @@ -802,7 +815,6 @@ def integrate_package_agents( ): files_skipped += 1 continue - target_path.parent.mkdir(parents=True, exist_ok=True) links_resolved = self.copy_agent(source_file, target_path) total_links_resolved += links_resolved files_integrated += 1 @@ -814,7 +826,6 @@ def integrate_package_agents( source_file, package_info.package.name, claude_target, - package_info.install_path, ) claude_path = claude_agents_dir / claude_filename try: @@ -834,7 +845,6 @@ def integrate_package_agents( elif not self.check_collision( claude_path, claude_rel, managed_files, force, diagnostics=diagnostics ): - claude_path.parent.mkdir(parents=True, exist_ok=True) self.copy_agent(source_file, claude_path) target_paths.append(claude_path) @@ -844,7 +854,6 @@ def integrate_package_agents( source_file, package_info.package.name, cursor_target, - package_info.install_path, ) cursor_path = cursor_agents_dir / cursor_filename try: @@ -864,7 +873,6 @@ def integrate_package_agents( elif not self.check_collision( cursor_path, cursor_rel, managed_files, force, diagnostics=diagnostics ): - cursor_path.parent.mkdir(parents=True, exist_ok=True) self.copy_agent(source_file, cursor_path) target_paths.append(cursor_path) diff --git a/tests/integration/test_marketplace_plugin_integration.py b/tests/integration/test_marketplace_plugin_integration.py index f6a210313f..81889f53e2 100644 --- a/tests/integration/test_marketplace_plugin_integration.py +++ b/tests/integration/test_marketplace_plugin_integration.py @@ -243,8 +243,8 @@ def test_plugin_detection_and_structure_mapping(self, tmp_path): "Command should be mapped to prompts" ) - def test_nested_agent_bundle_maps_and_deploys_to_multiple_targets(self, tmp_path): - """A declared agent bundle keeps identity and reports sibling resources.""" + def test_declared_agent_directory_flattens_agents_and_reports_resources(self, tmp_path): + """Legacy projection stays discoverable and reports unsupported resources.""" plugin_dir = tmp_path / "plugin" agent_dir = plugin_dir / "agents" / "my-agent" (agent_dir / "scripts").mkdir(parents=True) @@ -317,12 +317,12 @@ def test_nested_agent_bundle_maps_and_deploys_to_multiple_targets(self, tmp_path ) prepare_agent_files.assert_called_once() - assert (tmp_path / ".github/agents/my-agent/my-agent.agent.md").is_file() - assert (tmp_path / ".claude/agents/my-agent/my-agent.md").is_file() + assert (tmp_path / ".github/agents/my-agent.agent.md").is_file() + assert (tmp_path / ".claude/agents/my-agent.md").is_file() assert result["agents"] == 2 warnings = diagnostics.by_category()[CATEGORY_WARNING] assert len(warnings) == 1 - assert warnings[0].detail == ".apm/agents/my-agent/scripts/helper.py" + assert warnings[0].detail == ".apm/agents/scripts/helper.py" def test_plugin_with_dependencies(self, tmp_path): """Test plugin with dependencies are handled correctly.""" diff --git a/tests/unit/integration/test_agent_integrator.py b/tests/unit/integration/test_agent_integrator.py index 113ed54591..872e6a9941 100644 --- a/tests/unit/integration/test_agent_integrator.py +++ b/tests/unit/integration/test_agent_integrator.py @@ -563,8 +563,8 @@ def test_find_agent_files_discovers_nested_subdirectories(self): assert "nested.agent.md" in names assert "plain-nested.md" in names - def test_nested_agent_bundle_filters_resources_and_preserves_agent_path(self): - """Nested resources are not agents and nested agent identity is preserved.""" + def test_nested_agent_source_filters_resources_and_flattens_agent_path(self): + """Nested resources are rejected while legacy agent output stays flat.""" from apm_cli.integration.targets import KNOWN_TARGETS package_dir = self.project_root / "package" @@ -607,7 +607,7 @@ def test_nested_agent_bundle_filters_resources_and_preserves_agent_path(self): source_plan=source_plan, ) - expected = self.project_root / ".github" / "agents" / "my-agent" / "my-agent.agent.md" + expected = self.project_root / ".github" / "agents" / "my-agent.agent.md" assert result.target_paths == [expected] assert expected.is_file() assert not (self.project_root / ".github" / "agents" / "reference-doc.agent.md").exists() diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py index 44c63b2c47..7eebd18ea8 100644 --- a/tests/unit/test_plugin_parser.py +++ b/tests/unit/test_plugin_parser.py @@ -843,8 +843,8 @@ def test_custom_agents_dir_list_flattens_contents(self, tmp_path): "Should not create nested agents/agents/ directory" ) - def test_declared_agent_subdirectory_preserves_bundle_path(self, tmp_path): - """A declared directory under agents keeps its grouping and resources.""" + def test_declared_agent_subdirectory_flattens_into_legacy_staging(self, tmp_path): + """A declared agent directory is flattened for legacy primitive projection.""" plugin_dir = tmp_path / "plugin" agent_dir = plugin_dir / "agents" / "my-agent" (agent_dir / "guides").mkdir(parents=True) @@ -863,10 +863,11 @@ def test_declared_agent_subdirectory_preserves_bundle_path(self, tmp_path): manifest={"agents": ["./agents/my-agent"]}, ) - staged = apm_dir / "agents" / "my-agent" + staged = apm_dir / "agents" assert (staged / "my-agent.md").is_file() assert (staged / "guides" / "reference-doc.md").is_file() assert (staged / "scripts" / "helper.py").is_file() + assert not (staged / "my-agent").exists() class TestGenerateApmYml: From 5aa70868c77af774987b1afb12614f4703315ff4 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 1 Sep 2026 10:58:39 +0200 Subject: [PATCH 12/15] fix: preserve legacy plugin resource references Resolve the exact Claude plugin root token before projecting agents to harness discovery directories, while preserving recursive manifest semantics and target-specific rendering. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../owners/install-deployment.json | 7 - .../owners/marketplace-plugins.json | 3 +- .github/agents/algorithmic-patterns.agent.md | 152 ++++++++++++ .github/agents/workflows/perf-scan.md | 4 +- .github/workflows/perf-scan.md | 6 +- CHANGELOG.md | 6 +- apm.lock.yaml | 11 + .../instructions-and-agents.md | 29 ++- .../content/docs/reference/targets-matrix.md | 4 - .../skills/apm-usage/package-authoring.md | 20 +- .../checks/install_agent_inventory.py | 65 ----- .../checks/install_deployment_analyzers.py | 9 - .../marketplace_package_and_registration.py | 22 ++ scripts/architecture_linter/diagnostics.py | 1 - src/apm_cli/commands/uninstall/engine.py | 2 +- src/apm_cli/compilation/link_resolver.py | 11 + src/apm_cli/deps/plugin_parser.py | 8 +- src/apm_cli/install/deployable_source_plan.py | 6 +- src/apm_cli/install/primitive_integration.py | 38 --- src/apm_cli/install/services.py | 26 +- src/apm_cli/integration/agent_integrator.py | 233 +++++++----------- src/apm_cli/integration/base_integrator.py | 21 +- .../test_architecture_owner_rule_mutations.py | 10 +- .../test_install_services_orchestration.py | 27 -- .../test_integrators_validation_rules.py | 10 +- .../test_marketplace_plugin_integration.py | 88 ------- .../compilation/test_link_resolver_phase3.py | 32 +++ .../unit/install/test_security_scan_scope.py | 24 -- .../unit/integration/test_agent_integrator.py | 192 ++++++--------- .../unit/scripts/test_architecture_runner.py | 1 - tests/unit/test_plugin_parser.py | 41 +-- .../unit/test_surviving_deps_reintegration.py | 5 +- 32 files changed, 498 insertions(+), 616 deletions(-) create mode 100644 .github/agents/algorithmic-patterns.agent.md delete mode 100644 scripts/architecture_linter/checks/install_agent_inventory.py delete mode 100644 src/apm_cli/install/primitive_integration.py diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index d8e1b6c326..5fa44ca3da 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -50,13 +50,6 @@ "selectors": ["src/apm_cli/install/service.py"], "guards": ["install-deployment-frozen-mutation-eligibility"] }, - { - "id": "agent-source-admission-inventory", - "decision": "Agent source admission and package-level inventory", - "owner": "integration/agent_integrator.py (prepare_agent_files, _is_plain_md_agent)", - "selectors": ["src/apm_cli/integration/agent_integrator.py"], - "guards": ["install-deployment-agent-source-inventory"] - }, { "id": "authorized-deployable-source-paths", "decision": "Authorized deployable source paths", diff --git a/.apm/architecture/owners/marketplace-plugins.json b/.apm/architecture/owners/marketplace-plugins.json index 768608310c..34ae67542f 100644 --- a/.apm/architecture/owners/marketplace-plugins.json +++ b/.apm/architecture/owners/marketplace-plugins.json @@ -76,8 +76,9 @@ { "id": "legacy-plugin-skill-membership", "decision": "Legacy plugin declared-skill membership and plugin-root placeholder expansion", - "owner": "deps/plugin_parser.py (_map_plugin_artifacts, normalized_plugin_skill_sources, resolve_plugin_root_placeholders)", + "owner": "deps/plugin_parser.py (membership facade) and compilation/link_resolver.py (token expansion)", "selectors": [ + "src/apm_cli/compilation/link_resolver.py", "src/apm_cli/deps/plugin_parser.py", "src/apm_cli/integration/skill_integrator.py" ], diff --git a/.github/agents/algorithmic-patterns.agent.md b/.github/agents/algorithmic-patterns.agent.md new file mode 100644 index 0000000000..bd4e4afac8 --- /dev/null +++ b/.github/agents/algorithmic-patterns.agent.md @@ -0,0 +1,152 @@ +# Algorithmic Performance Patterns + +Load this reference when the PR diff touches code outside the +transport/cache layer -- i.e. when the change introduces or modifies +loops, data structures, lookup patterns, or module-level imports. + +## Big O Quick Reference + +| Pattern | Complexity | Red Flag | +|---------|-----------|----------| +| Dict/set lookup | O(1) | Fine | +| List `.append` | O(1) amortised | Fine | +| `x in list` | O(n) | Use a set if called in a loop | +| Nested loops over same collection | O(n^2) | Extract an index dict first | +| Sort inside a loop | O(n^2 log n) | Sort once outside the loop | +| `any(pred(x) for x in coll)` in a loop | O(n*m) | Build a set/dict pre-loop | +| Unconditional full-dir scan on every write | O(n) per write = O(n^2) total | Track running total; scan only when needed | +| Linear search for identity match | O(n) per lookup | Build `{identity: index}` once | + +## Anti-Patterns to Flag + +### 1. Missing Index on Repeated Lookup + +```python +# BAD: O(n) per call, called m times = O(n*m) +def has_item(collection, key): + return any(item.key == key for item in collection) + +# GOOD: O(1) per call after O(n) setup +_index = {item.key for item in collection} +def has_item(key): + return key in _index +``` + +Flag when: a function does linear scan AND is called from within a +loop or from a method called repeatedly during resolution/install. + +### 2. Unconditional Expensive Operation + +```python +# BAD: scans entire cache dir on every store() +def store(self, url, body): + self._write(url, body) + self._enforce_size_cap() # full scandir every time + +# GOOD: fast-path skip when clearly under budget +def store(self, url, body): + self._write(url, body) + self._tracked_size += len(body) + if self._tracked_size > MAX_SIZE: + self._enforce_size_cap() # scan only when needed +``` + +Flag when: an expensive operation (directory walk, sort, full +re-computation) runs unconditionally on every call to a high-frequency +method. + +### 3. Triple-Pass Where Single-Pass Suffices + +```python +# BAD: iterates refs 3x (once per category) +for ref in refs: + if ref.startswith("refs/tags/"): ... +for ref in refs: + if ref.name == target: ... +for ref in refs: + if ref.name == f"refs/heads/{target}": ... + +# GOOD: single pass builds lookup dicts +tags, branches, by_name = {}, {}, {} +for ref in refs: + by_name[ref.name] = ref + if ref.name.startswith("refs/tags/"): + tags[strip_prefix(ref.name)] = ref + elif ref.name.startswith("refs/heads/"): + branches[ref.name[len("refs/heads/"):]] = ref +# Then O(1) lookups +``` + +Flag when: the same collection is iterated multiple times with +different predicates that could all be evaluated in one pass. + +### 4. Repeated Environment/Config Parsing + +```python +# BAD: re-parses on every call +def classify_host(hostname): + ghes = os.environ.get("GITHUB_HOST", "").strip().lower().split("/")[0] + # ... repeated in 5 other functions + +# GOOD: parse once in a helper +def _get_ghes_host(): + return os.environ.get("GITHUB_HOST", "").strip().lower().split("/")[0] +``` + +Flag when: the same `os.environ.get()` + normalisation chain appears +in multiple functions that are called in tight succession. + +### 5. Heavy Top-Level Imports on CLI Startup + +```python +# BAD: imports entire install engine at module level +from apm_cli.install.pipeline import FullPipeline # 40+ transitive modules + +# GOOD: defer to function scope +def install_command(): + from apm_cli.install.pipeline import FullPipeline + ... +``` + +Flag when: a command module imports heavy subpackages at the top level +that are not needed for other commands sharing the same CLI entrypoint. + +### 6. Synchronous Blocking in Parallelisable Paths + +```python +# BAD: sequential I/O in a loop +for pkg in packages: + metadata = fetch_metadata(pkg) # blocking network call + +# GOOD: parallel with bounded concurrency +with ThreadPoolExecutor(max_workers=8) as pool: + metadata_list = list(pool.map(fetch_metadata, packages)) +``` + +Flag when: a loop performs independent I/O operations (file reads, +network calls, subprocess spawns) that have no data dependency between +iterations. + +## Scaling Guard Pattern + +When reviewing a fix, recommend a scaling-guard test: +- Run the operation at size N and at size 10*N +- Assert the time ratio stays below a threshold (e.g. < 15x) +- This catches O(n^2) regressions without brittle absolute-time assertions + +```python +def test_scaling_ratio(): + t_small = median_time(lambda: operation(n=50)) + t_large = median_time(lambda: operation(n=500)) + ratio = t_large / t_small + assert ratio < 15, f"Ratio {ratio:.1f}x suggests O(n^2)" +``` + +## Quantification Checklist + +When reporting a performance finding, always state: +1. **What** -- the specific pattern (name it from the table above) +2. **Where** -- file:line range +3. **Frequency** -- how often this code path executes per typical run +4. **Complexity** -- the current Big O and the proposed Big O +5. **Fix** -- concrete code sketch (not just "optimise this") diff --git a/.github/agents/workflows/perf-scan.md b/.github/agents/workflows/perf-scan.md index 5ab5caa8cc..2c19b81ab2 100644 --- a/.github/agents/workflows/perf-scan.md +++ b/.github/agents/workflows/perf-scan.md @@ -17,7 +17,7 @@ a GitHub Issue on every run so the team has a daily record. The agent operates as the `performance-expert` persona defined in `.github/agents/performance-expert.agent.md`. When scanning non-transport code (i.e., anything outside `src/apm_cli/transport/`), it also loads the -pattern catalogue from `.apm/agents/references/algorithmic-patterns.md`. +pattern catalogue from `.github/agents/algorithmic-patterns.agent.md`. ## Anti-patterns checked @@ -67,5 +67,5 @@ This ensures the daily run is always visible in the issue tracker. - Workflow definition: `.github/workflows/perf-scan.md` - Performance agent: `.github/agents/performance-expert.agent.md` -- Pattern catalogue: `.apm/agents/references/algorithmic-patterns.md` +- Pattern catalogue: `.github/agents/algorithmic-patterns.agent.md` - Benchmark examples: `tests/benchmarks/test_perf_benchmarks.py` diff --git a/.github/workflows/perf-scan.md b/.github/workflows/perf-scan.md index 7cf1ecf08b..adc3c164ea 100644 --- a/.github/workflows/perf-scan.md +++ b/.github/workflows/perf-scan.md @@ -39,7 +39,7 @@ actionable findings. Read `.github/agents/performance-expert.agent.md` for your full persona and mental model. When scanning non-transport code, also load -`.apm/agents/references/algorithmic-patterns.md` for the pattern catalogue. +`.github/agents/algorithmic-patterns.agent.md` for the pattern catalogue. ## Context @@ -59,8 +59,8 @@ Before scanning, read the algorithmic patterns reference so you know exactly wha to look for: ```bash -cat .apm/agents/references/algorithmic-patterns.md 2>/dev/null || \ - echo "[!] algorithmic-patterns.md not found -- using inline patterns" +cat .github/agents/algorithmic-patterns.agent.md 2>/dev/null || \ + echo "[!] algorithmic-patterns.agent.md not found -- using inline patterns" ``` Then read a sample of the source tree structure: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5631fe5c6a..5ca1c85ecc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Legacy Claude plugin agents now resolve `${CLAUDE_PLUGIN_ROOT}` to their + retained package under `apm_modules` across all projected targets, including + Codex and Kiro. (closes #2692) - Distributed `apm compile` now reconciles existing managed-section `AGENTS.md` files without overwriting hand-authored content, generates new placements safely, and never discovers, writes, or cleans content across @@ -47,9 +50,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm doctor` now reports malformed project executable-trust configuration under either `executables` or the deprecated `allowExecutables` key as an actionable informational warning instead of omitting the check. (#2719) -- Legacy agent projection now stays flat for harness discovery, admits plain - Markdown only with non-empty `name` and `description` frontmatter, and emits - one actionable warning when it skips unsupported sibling resources. (#2721) - Git subdirectory dependencies with symlinks to files elsewhere in the same repository now install successfully where Git materializes symlinks; APM diff --git a/apm.lock.yaml b/apm.lock.yaml index 27ebf673d0..1c4699878d 100644 --- a/apm.lock.yaml +++ b/apm.lock.yaml @@ -2595,6 +2595,15 @@ deployments: - . active_owner: . content_hash: sha256:d1ea2d038e2af8be11d6c95b3213b03b9777fae46f0438efa95d5a803e6c3765 +- kind: project-relative + target: copilot + value: .github/agents/algorithmic-patterns.agent.md + runtime: null + scope: project + owners: + - . + active_owner: . + content_hash: sha256:278321484288eb8965a42c20cfa65ff055d250579849b1c48cbd3f553883fe5d - kind: project-relative target: copilot value: .github/agents/apm-ceo.agent.md @@ -3011,6 +3020,7 @@ local_deployed_files: - .agents/skills/supply-chain-security - .agents/skills/supply-chain-security/SKILL.md - .github/agents/agentic-workflows.agent.md +- .github/agents/algorithmic-patterns.agent.md - .github/agents/apm-ceo.agent.md - .github/agents/apm-primitives-architect.agent.md - .github/agents/auth-expert.agent.md @@ -3260,6 +3270,7 @@ local_deployed_file_hashes: .agents/skills/python-architecture/SKILL.md: sha256:f06e50b8c40d7e5a732a5405a603efd80da58de2b6f6b293f0f6b58f28eefe7b .agents/skills/supply-chain-security/SKILL.md: sha256:55ef10cd0f6b68e0db5a435a79851c055eab74d93fd552c6626df631deb51df4 .github/agents/agentic-workflows.agent.md: sha256:d1ea2d038e2af8be11d6c95b3213b03b9777fae46f0438efa95d5a803e6c3765 + .github/agents/algorithmic-patterns.agent.md: sha256:278321484288eb8965a42c20cfa65ff055d250579849b1c48cbd3f553883fe5d .github/agents/apm-ceo.agent.md: sha256:484da64428ea46a6183dffd3f30c9fc5fc5c747639c0c79e55be69dba0899323 .github/agents/apm-primitives-architect.agent.md: sha256:0c8f04297b14e144f84211b5b881b099233a9847c0fca257b9aa69c4d43d1eef .github/agents/auth-expert.agent.md: sha256:18264a933cba432b77d133e6ae11eee294c92ed245629af8c9b7a5bb7a9a300c diff --git a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md index 531b4936c5..c311630bd1 100644 --- a/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md +++ b/docs/src/content/docs/producer/author-primitives/instructions-and-agents.md @@ -139,18 +139,23 @@ my-package/ File names end in `.agent.md` and live under `.apm/agents/`. -For flat-file harnesses, legacy integration flattens nested definitions into -the target's agents directory because those harnesses may not discover nested -agents. Plain `.md` files under `.apm/agents/` are agents only when YAML -frontmatter contains non-empty `name` and `description` fields. APM skips other -Markdown and non-Markdown sibling resources and emits one actionable warning; -use `--verbose` to list the paths. Package runtime resources in a skill bundle. - -:::note[Planned] -Agent Plugins 1.0 targets full bundle preservation. Today APM preserves bundles -natively only for Copilot, and only for skills and MCP servers. See -[Pack a bundle](../../pack-a-bundle/#what-apm-pack-produces). -::: +### Legacy Claude plugin bundles + +APM retains the complete downloaded legacy Claude plugin under `apm_modules` and +projects its agents into each target's discovery directory. For legacy targets, +APM resolves Claude's official `${CLAUDE_PLUGIN_ROOT}` token in agent content to +the retained package. Inline Markdown links to existing package assets are also +rewritten. + +A manifest-declared agent directory recursively treats every Markdown file as +an agent. If it also contains supporting Markdown that should not be invokable, +declare exact agent files: + +```json +{ + "agents": ["./agents/my-agent/my-agent.md"] +} +``` ### Frontmatter diff --git a/docs/src/content/docs/reference/targets-matrix.md b/docs/src/content/docs/reference/targets-matrix.md index a78aa87d80..72037d4bc6 100644 --- a/docs/src/content/docs/reference/targets-matrix.md +++ b/docs/src/content/docs/reference/targets-matrix.md @@ -128,10 +128,6 @@ GitHub Copilot (CLI and IDE). CLI can invoke them from any working directory. - **Global compile.** `apm compile -g` can also render global instructions to `~/.copilot/AGENTS.md` for root-context readers that honor `AGENTS.md`. -:::note[Planned] -Agent Plugins 1.0 targets full bundle preservation. Today APM preserves bundles -natively only for Copilot, and only for skills and MCP servers. -::: ## claude diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index 33cd3390ee..6a896dc451 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -14,6 +14,13 @@ how to install it: | `plugin.json` (no `$schema`) / `.claude-plugin/` | Claude plugin collection | Dissect via plugin artifact mapping | | `plugin.json` with an Agent Plugins `$schema` | Portable Agent Plugin | Acquired and locked as one opaque unit; registered when effective targets include Copilot and admission gates pass. Excluded targets create no native registration or loose primitive projection. APM does not require the runtime during lifecycle operations; loading requires supported Copilot CLI 1.0.81 or newer | +Legacy Claude plugin collections remain materialized under `apm_modules`; APM +projects their agents into target discovery directories. For legacy targets, +APM resolves Claude's official `${CLAUDE_PLUGIN_ROOT}` token and rewrites inline +Markdown links to existing package assets. A manifest-declared agent directory +recursively treats every Markdown file as an agent. Declare exact agent files +for mixed directories. + For Agent Plugins with the same declared name, a direct dependency wins over a transitive dependency. APM refuses same-precedence collisions and does not silently repoint a ledger-recorded owner to a transitive claimant. @@ -341,17 +348,6 @@ supported top-level harness directories: `.agents`, `.apm`, `.claude`, Chat persona configuration. Place in `.apm/agents/`. -Legacy integration flattens nested agent definitions for flat-file harnesses -because they may not discover nested agents. Plain `.md` -files under `.apm/agents/` must declare non-empty `name` and `description` -fields in YAML frontmatter. Other Markdown and non-Markdown sibling resources -are skipped with one actionable warning; use `--verbose` to list the paths. -Package runtime resources in a skill bundle. - -Full bundle preservation is planned for Agent Plugins 1.0; it is not current -legacy projection behavior. Today APM preserves bundles natively only for -Copilot, and only for skills and MCP servers. - ```yaml --- name: "architect" @@ -879,7 +875,7 @@ is a hard error -- run `migrate` to consolidate. ### Full guide -See [docs/guides/marketplace-authoring](../../../../../docs/src/content/docs/guides/marketplace-authoring.md) +See the [marketplace publishing guide](../../../../../docs/src/content/docs/producer/publish-to-a-marketplace.md) for the complete maintainer workflow (quickstart, version ranges, `check`, `doctor`, and `outdated`). diff --git a/scripts/architecture_linter/checks/install_agent_inventory.py b/scripts/architecture_linter/checks/install_agent_inventory.py deleted file mode 100644 index f92d813eb8..0000000000 --- a/scripts/architecture_linter/checks/install_agent_inventory.py +++ /dev/null @@ -1,65 +0,0 @@ -"""Agent source admission and package-inventory architecture guard.""" - -from __future__ import annotations - -from scripts.architecture_linter.checks.install_deployment_shared import ( - _facts_for, - _present, - _python_paths, - _summary, -) -from scripts.architecture_linter.facts import FactsProvider -from scripts.architecture_linter.models import Violation - -_GUARD_AGENT_SOURCE_INVENTORY = "install-deployment-agent-source-inventory" -_OWNER = "src/apm_cli/integration/agent_integrator.py" -_PREPARATION = "src/apm_cli/install/primitive_integration.py" -_SERVICES = "src/apm_cli/install/services.py" -_OWNER_DEFINITIONS = frozenset({"_is_plain_md_agent", "prepare_agent_files"}) - - -def check_agent_source_inventory(provider: FactsProvider) -> tuple[Violation, ...]: - """Agent admission and inventory must route through AgentIntegrator.""" - rule_id = _GUARD_AGENT_SOURCE_INVENTORY - owner, owner_fail = _facts_for(provider, _OWNER, rule_id) - preparation, preparation_fail = _facts_for(provider, _PREPARATION, rule_id) - services, services_fail = _facts_for(provider, _SERVICES, rule_id) - if owner_fail or preparation_fail or services_fail: - return tuple(list(owner_fail) + list(preparation_fail) + list(services_fail)) - - definition_counts = dict.fromkeys(_OWNER_DEFINITIONS, 0) - for path in _python_paths(provider, "src/apm_cli/"): - facts, failures = _facts_for(provider, path, rule_id) - if failures: - return tuple(failures) - for definition in facts.definitions: - if definition.name in definition_counts: - definition_counts[definition.name] += 1 - - required_owner_fragments = ( - "files, _ignored = self._classify_agent_files(package_path)", - "agent_files, ignored_resources = self._classify_agent_files(package_path)", - "frontmatter = load_frontmatter(str(source)).metadata", - 'name = frontmatter.get("name")', - 'description = frontmatter.get("description")', - "and bool(name.strip())", - "and bool(description.strip())", - "if agent_files is None:", - ) - if ( - any(count != 1 for count in definition_counts.values()) - or any(not _present(owner, fragment) for fragment in required_owner_fragments) - or not _present(preparation, '"agent_files": integrator.prepare_agent_files(') - or not _present(services, "prepare_primitive_inputs as _prepare_primitive_inputs") - ): - return ( - _summary( - rule_id, - _OWNER, - "Agent admission and inventory must route through AgentIntegrator", - ), - ) - return () - - -__all__ = ["_GUARD_AGENT_SOURCE_INVENTORY", "check_agent_source_inventory"] diff --git a/scripts/architecture_linter/checks/install_deployment_analyzers.py b/scripts/architecture_linter/checks/install_deployment_analyzers.py index 106620423d..3acdc3237e 100644 --- a/scripts/architecture_linter/checks/install_deployment_analyzers.py +++ b/scripts/architecture_linter/checks/install_deployment_analyzers.py @@ -13,10 +13,6 @@ from __future__ import annotations -from scripts.architecture_linter.checks.install_agent_inventory import ( - _GUARD_AGENT_SOURCE_INVENTORY, - check_agent_source_inventory, -) from scripts.architecture_linter.checks.install_base_integrator_and_contraction import ( _GUARD_BASE_INTEGRATOR, _GUARD_PROVENANCE, @@ -58,11 +54,6 @@ from scripts.architecture_linter.models import Rule RULES: tuple[Rule, ...] = ( - _rule( - _GUARD_AGENT_SOURCE_INVENTORY, - "Agent admission and inventory stay owned by AgentIntegrator.", - check_agent_source_inventory, - ), _rule( _GUARD_PACKAGE_TARGET, "Restriction-only package target authorization has one owner (install/target_filter.py).", diff --git a/scripts/architecture_linter/checks/marketplace_package_and_registration.py b/scripts/architecture_linter/checks/marketplace_package_and_registration.py index 6fb310d471..b8b7183ad4 100644 --- a/scripts/architecture_linter/checks/marketplace_package_and_registration.py +++ b/scripts/architecture_linter/checks/marketplace_package_and_registration.py @@ -20,6 +20,7 @@ _def_body_text, _forbid_scan, _load, + _require_res, _require_subs, _src_python, _subdir_python, @@ -204,6 +205,7 @@ def _scan_normalization_callers( _SKILL_INTEGRATOR = "src/apm_cli/integration/skill_integrator.py" +_PLUGIN_ROOT_OWNER = "src/apm_cli/compilation/link_resolver.py" _SKILL_SUBSET_LEXICAL = re.compile( @@ -228,6 +230,26 @@ def _check_legacy_skill_membership(provider: FactsProvider) -> tuple[Violation, "plugin_parser must own skill membership and artifact mapping", ) ) + findings.extend( + _require_res( + provider, + inv, + _RID_SKILL, + _PLUGIN_ROOT_OWNER, + (re.compile(r"^def resolve_claude_plugin_root\("),), + "link_resolver must own Claude plugin-root token expansion", + ) + ) + findings.extend( + _require_subs( + provider, + inv, + _RID_SKILL, + _PLUGIN_PARSER, + ("resolve_claude_plugin_root(value, plugin_path)",), + "plugin_parser must delegate plugin-root expansion to link_resolver", + ) + ) integrator_facts, failures = _load(provider, inv, _RID_SKILL, _SKILL_INTEGRATOR, parse=True) if failures: diff --git a/scripts/architecture_linter/diagnostics.py b/scripts/architecture_linter/diagnostics.py index f0f4c6bfe3..b74a4d6be9 100644 --- a/scripts/architecture_linter/diagnostics.py +++ b/scripts/architecture_linter/diagnostics.py @@ -36,7 +36,6 @@ "contracts-tooling-dependency-identity": ("AC23", "AC25", "AC29"), "contracts-tooling-frontmatter-yaml": ("AC36",), "install-deployment-approval-outcome-routing": ("AC3",), - "install-deployment-agent-source-inventory": ("AC37",), "install-deployment-audit-policy-discovery": ("AC3",), "install-deployment-audit-replay": ("AC4",), "install-deployment-cached-claude-skill-metadata": ("AC4",), diff --git a/src/apm_cli/commands/uninstall/engine.py b/src/apm_cli/commands/uninstall/engine.py index e3b060c4cb..ef224165e6 100644 --- a/src/apm_cli/commands/uninstall/engine.py +++ b/src/apm_cli/commands/uninstall/engine.py @@ -1295,7 +1295,7 @@ def _sync_integrations_after_uninstall( _rebuild_scope = InstallScope.USER if user_scope else InstallScope.PROJECT _allow_executables = getattr(apm_package, "allow_executables", None) - reintegration_diagnostics = DiagnosticCollector(verbose=logger.verbose) + reintegration_diagnostics = DiagnosticCollector() for dep_ref, pkg_info, authorized_targets in target_survivor_plan: dep_key = dep_ref.get_unique_key() deployed_files = package_deployed_files.setdefault(dep_key, []) diff --git a/src/apm_cli/compilation/link_resolver.py b/src/apm_cli/compilation/link_resolver.py index 2414c79820..72154375d8 100644 --- a/src/apm_cli/compilation/link_resolver.py +++ b/src/apm_cli/compilation/link_resolver.py @@ -17,6 +17,14 @@ from apm_cli.utils.path_security import PathTraversalError, ensure_path_within from apm_cli.utils.paths import portable_link_relpath +CLAUDE_PLUGIN_ROOT = "${CLAUDE_PLUGIN_ROOT}" + + +def resolve_claude_plugin_root(content: str, plugin_root: Path) -> str: + """Expand Claude's exact plugin-root token to the materialized package.""" + return content.replace(CLAUDE_PLUGIN_ROOT, plugin_root.resolve().as_posix()) + + # CRITICAL: Shadow Click commands to prevent namespace collision set = builtins.set list = builtins.list @@ -80,6 +88,7 @@ def __init__(self, base_dir: Path): # the generalization safely. self.package_root: Path | None = None self.deployment_package_root: Path | None = None + self.claude_plugin_root: Path | None = None def register_contexts(self, primitives) -> None: """Build registry of all available context files. @@ -145,6 +154,8 @@ def resolve_links_for_installation( deployment_package_root=self.deployment_package_root, ) + if self.claude_plugin_root is not None: + content = resolve_claude_plugin_root(content, self.claude_plugin_root) return self._rewrite_markdown_links(content, ctx) def resolve_links_for_compilation( diff --git a/src/apm_cli/deps/plugin_parser.py b/src/apm_cli/deps/plugin_parser.py index 817292de3b..b7e625498f 100644 --- a/src/apm_cli/deps/plugin_parser.py +++ b/src/apm_cli/deps/plugin_parser.py @@ -763,7 +763,9 @@ def _substitute_plugin_root( def resolve_plugin_root_placeholders(value: Any, plugin_path: Path) -> Any: """Resolve plugin-root placeholders in an in-memory manifest value.""" if isinstance(value, str): - return value.replace("${CLAUDE_PLUGIN_ROOT}", str(plugin_path.resolve())) + from apm_cli.compilation.link_resolver import resolve_claude_plugin_root + + return resolve_claude_plugin_root(value, plugin_path) if isinstance(value, dict): return { key: resolve_plugin_root_placeholders(item, plugin_path) for key, item in value.items() @@ -1149,8 +1151,8 @@ def _is_same_path(src: Path, dst: Path) -> bool: # Map agents/ # Unlike skills (which are named directories containing SKILL.md), agents - # are flat files -- each admitted Markdown file is one agent. Always merge - # directory contents directly into .apm/agents/. + # are flat files -- each .md is one agent. So we always merge directory + # contents directly into .apm/agents/ (no nesting by dir name). agent_sources = _resolve_sources("agents", "agents") if agent_sources: target_agents = apm_dir / "agents" diff --git a/src/apm_cli/install/deployable_source_plan.py b/src/apm_cli/install/deployable_source_plan.py index 73cd606fb4..1f2b04340e 100644 --- a/src/apm_cli/install/deployable_source_plan.py +++ b/src/apm_cli/install/deployable_source_plan.py @@ -118,10 +118,8 @@ def add_direct_matching_files(root: Path, pattern: str) -> None: add_matching_files(source_root / ".apm" / "prompts", "*.prompt.md") if "agents" in target_primitives: - from apm_cli.integration.agent_integrator import AgentIntegrator - - for path in AgentIntegrator().find_agent_files(source_root): - add_file(path) + add_direct_matching_files(source_root, "*.agent.md") + add_matching_files(source_root / ".apm" / "agents", "*.md") if "instructions" in target_primitives: add_matching_files(source_root / ".apm" / "instructions", "*.instructions.md") diff --git a/src/apm_cli/install/primitive_integration.py b/src/apm_cli/install/primitive_integration.py deleted file mode 100644 index d995c1cc51..0000000000 --- a/src/apm_cli/install/primitive_integration.py +++ /dev/null @@ -1,38 +0,0 @@ -"""Package-scoped preparation and presentation for primitive integration.""" - -from __future__ import annotations - -from typing import Any - - -def prepare_primitive_inputs( - primitive_name: str, - integrator: Any, - package_info: Any, - targets: Any, - diagnostics: Any, - source_plan: Any, -) -> dict[str, Any]: - """Prepare package-scoped inputs reused across target integrations.""" - if primitive_name != "agents" or not any( - target.primitives.get("agents") is not None for target in targets - ): - return {} - return { - "agent_files": integrator.prepare_agent_files( - package_info.install_path, - package_info.package.name, - diagnostics, - source_plan, - ) - } - - -def emit_integration_hints(primitive_name: str, info: dict, log_integration) -> None: - """Emit user actions that follow successful primitive integration.""" - if any(path.startswith("copilot-app/") for path in info["paths"]) and info["files"] > 0: - log_integration( - " |-- workflows arrive disabled; enable from the Copilot App's Workflows tab" - ) - if primitive_name == "canvas" and (info["files"] > 0 or info["adopted"] > 0): - log_integration(" |-- reload the Copilot session (/clear) or restart to load the canvas") diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index c3ce000c6c..168c6d0ddd 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -35,8 +35,6 @@ from .local_bundle_paths import bundle_slug_validation_error as _bundle_slug_error from .local_bundle_paths import known_bundle_deploy_prefixes as _known_bundle_prefixes from .local_bundle_paths import target_bundle_deploy_prefixes as _target_bundle_prefixes -from .primitive_integration import emit_integration_hints as _emit_integration_hints -from .primitive_integration import prepare_primitive_inputs as _prepare_primitive_inputs from .target_filter import resolve_effective_package_targets if TYPE_CHECKING: @@ -117,6 +115,21 @@ def _label_and_deploy_dir(prim_name: str, mapping, target, deploy_dir: str) -> t return prim_name, deploy_dir +def _emit_integration_hints(prim_name: str, info: dict, log_integration) -> None: + """Emit per-primitive 'next step' hints after an integration line.""" + # copilot-app workflows arrive disabled: the row lands enabled=0 and the + # user must flip the toggle in the Copilot App's Workflows tab before the + # schedule fires. + if any(p.startswith("copilot-app/") for p in info["paths"]) and info["files"] > 0: + log_integration( + " |-- workflows arrive disabled; enable from the Copilot App's Workflows tab" + ) + # Canvas extensions are discovered by Copilot CLI at session start, so a + # freshly-deployed canvas is not picked up mid-session. + if prim_name == "canvas" and (info["files"] > 0 or info["adopted"] > 0): + log_integration(" |-- reload the Copilot session (/clear) or restart to load the canvas") + + def _log_hooks_skip( package_name: str, package_info: Any, targets: Any, logger: InstallLogger | None ) -> None: @@ -487,14 +500,6 @@ def _format_target_collapse(paths: list[str], verbose: bool) -> tuple[str, list[ _agg_paths: list[str] = [] _agg_hook_payloads: list = [] _label = _prim_name - _prepared_inputs = _prepare_primitive_inputs( - _prim_name, - _integrator, - package_info, - targets, - diagnostics, - source_plan, - ) for _target in targets: _mapping = _target.primitives.get(_prim_name) if _mapping is None: @@ -505,7 +510,6 @@ def _format_target_collapse(paths: list[str], verbose: bool) -> tuple[str, list[ "diagnostics": diagnostics, "scope": scope, "source_plan": source_plan, - **_prepared_inputs, } # Hook integrator alone needs the scope signal: project-scope # deploys keep ``command`` paths repo-relative (#1394), user-scope diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index 0f42b2e9a8..6e41ec9bfd 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -15,12 +15,11 @@ from apm_cli.integration.base_integrator import BaseIntegrator, IntegrationResult from apm_cli.integration.opencode_frontmatter import validate_opencode_frontmatter -from apm_cli.utils.atomic_io import normalize_crlf_to_lf, write_text_lf -from apm_cli.utils.console import _rich_warning +from apm_cli.utils.atomic_io import write_text_lf from apm_cli.utils.diagnostics import printable_ascii_text from apm_cli.utils.path_security import PathTraversalError, ensure_path_within from apm_cli.utils.paths import portable_relpath -from apm_cli.utils.yaml_io import load_frontmatter, load_yaml_str, yaml_to_str +from apm_cli.utils.yaml_io import load_yaml_str, yaml_to_str if TYPE_CHECKING: from apm_cli.integration.targets import TargetProfile @@ -44,7 +43,6 @@ "*", } ) -_IGNORED_AGENT_RESOURCE_DETAIL_LIMIT = 20 class AgentIntegrator(BaseIntegrator): @@ -58,8 +56,7 @@ def find_agent_files(self, package_path: Path, source_plan=None) -> list[Path]: Searches in: - Package root directory (*.agent.md files) - - .apm/agents/ subdirectory (recursive): explicit *.agent.md files - and plain *.md files with agent frontmatter + - .apm/agents/ subdirectory (recursive): *.agent.md and plain *.md files Args: package_path: Path to the package directory @@ -67,96 +64,18 @@ def find_agent_files(self, package_path: Path, source_plan=None) -> list[Path]: Returns: List[Path]: List of absolute paths to agent files """ - files, _ignored = self._classify_agent_files(package_path) - return self.filter_authorized_files(files, source_plan) - - def _classify_agent_files(self, package_path: Path) -> tuple[list[Path], list[Path]]: - """Classify package agent files and ignored sibling resources once.""" - files = self.find_files_by_glob(package_path, "*.agent.md") - ignored: list[Path] = [] + files: list[Path] = [] + # Flat search in package root + files += self.find_files_by_glob(package_path, "*.agent.md") + # Recursive search in .apm/agents/ (use ** glob for subdirectories) apm_agents = package_path / ".apm" / "agents" if apm_agents.exists(): - for path in self.find_files_by_glob(apm_agents, "**/*"): - if not path.is_file(): - continue - if path.name.endswith(".agent.md") or ( - path.suffix == ".md" and self._is_plain_md_agent(path) - ): - files.append(path) - else: - ignored.append(path) - return files, ignored - - @staticmethod - def _is_plain_md_agent(source: Path) -> bool: - """Return whether a plain Markdown file declares agent frontmatter.""" - try: - frontmatter = load_frontmatter(str(source)).metadata - except (OSError, UnicodeError, yaml.YAMLError): - return False - if not isinstance(frontmatter, dict): - return False - name = frontmatter.get("name") - description = frontmatter.get("description") - return ( - isinstance(name, str) - and bool(name.strip()) - and isinstance(description, str) - and bool(description.strip()) - ) - - def _warn_ignored_agent_resources( - self, - package_path: Path, - package_name: str, - ignored_resources: list[Path], - diagnostics=None, - ) -> None: - """Warn when files under .apm/agents are not deployable agents.""" - if not ignored_resources: - return - relative = sorted( - printable_ascii_text(path.relative_to(package_path).as_posix()) - for path in ignored_resources - ) - noun = "file" if len(relative) == 1 else "files" - message = ( - f"Ignored {len(relative)} non-agent {noun} under .apm/agents; " - "only *.agent.md files and plain Markdown files with name and " - "description frontmatter are deployable. Package required runtime " - "resources as a skill bundle, then rerun 'apm install'." - ) - if diagnostics is not None: - safe_package = printable_ascii_text(package_name) - visible = relative[:_IGNORED_AGENT_RESOURCE_DETAIL_LIMIT] - omitted = len(relative) - len(visible) - detail = ", ".join(visible) - if omitted: - detail = f"{detail}, ... (+{omitted} more)" - diagnostics.warn( - message=f"{message} Ignored file paths appear in verbose output.", - package=safe_package, - detail=detail, - ) - else: - _rich_warning(message) - - def prepare_agent_files( - self, - package_path: Path, - package_name: str, - diagnostics=None, - source_plan=None, - ) -> list[Path]: - """Discover deployable agents and report ignored resources once.""" - agent_files, ignored_resources = self._classify_agent_files(package_path) - self._warn_ignored_agent_resources( - package_path, - package_name, - ignored_resources, - diagnostics, - ) - return self.filter_authorized_files(agent_files, source_plan) + files += self.find_files_by_glob(apm_agents, "**/*.agent.md") + # Also pick up plain .md files; the directory name implies type + for f in self.find_files_by_glob(apm_agents, "**/*.md"): + if not f.name.endswith(".agent.md") and f not in files: + files.append(f) + return self.filter_authorized_files(files, source_plan) # NOTE: find_skill_file(), integrate_skill(), and _generate_skill_agent_content() # have been REMOVED as part of T5 (skill-strategy.md). @@ -193,7 +112,6 @@ def integrate_agents_for_target( diagnostics=None, scope=None, source_plan=None, - agent_files: list[Path] | None = None, ) -> IntegrationResult: """Integrate agents from a package for a single *target*. @@ -211,13 +129,7 @@ def integrate_agents_for_target( return IntegrationResult(0, 0, 0, []) self.init_link_resolver(package_info, project_root) - if agent_files is None: - agent_files = self.prepare_agent_files( - package_info.install_path, - package_info.package.name, - diagnostics, - source_plan, - ) + agent_files = self.find_agent_files(package_info.install_path, source_plan) if not agent_files: return IntegrationResult(0, 0, 0, []) @@ -233,8 +145,7 @@ def integrate_agents_for_target( total_links_resolved = 0 for source_file in agent_files: - # Kiro uses the relative source path as agent identity. Other - # harnesses discover flat files directly under their agents root. + # kiro_agent uses relative path from .apm/agents/ for identity. if mapping.format_id == "kiro_agent": target_relpath = self._kiro_agent_relpath(source_file, package_info.install_path) else: @@ -260,12 +171,20 @@ def integrate_agents_for_target( files_skipped += 1 continue + if source_file.is_symlink(): + raise ValueError(f"Refusing to read symlink source: {source_file}") + source_content = source_file.read_text(encoding="utf-8") + resolved_content, links_resolved = self.resolve_links( + source_content, source_file, target_path + ) + if mapping.format_id == "kiro_agent": # req-tg-009: Preflight render+validate MUST run before any # content-identity adoption fast-path or filesystem mutation. # If validation fails, skip without writing or creating dirs. rendered, ok = self._preflight_render_kiro_agent( source_file, + content=resolved_content, diagnostics=diagnostics, package_name=package_info.package.name, ) @@ -273,24 +192,22 @@ def integrate_agents_for_target( files_skipped += 1 continue - # Compare rendered artifact (not raw source) against existing - # target so a pre-placed file with invalid tools cannot be - # adopted by matching source bytes. rel_path = portable_relpath(target_path, project_root) - if target_path.exists() and not target_path.is_symlink(): - try: - existing = target_path.read_bytes() - rendered_bytes = normalize_crlf_to_lf(rendered).encode("utf-8") - if existing == rendered_bytes: - target_paths.append(target_path) - files_adopted += 1 - continue - except OSError: - pass - if self.check_collision( - target_path, rel_path, managed_files, force, diagnostics=diagnostics - ): - files_skipped += 1 + skip, adopted = self._check_adopt_or_skip( + target_path, + source_file, + rel_path, + managed_files, + force, + diagnostics, + target_paths, + expected_content=rendered, + ) + if skip: + if adopted: + files_adopted += 1 + else: + files_skipped += 1 continue # Safe to materialize: ensure parent dirs exist then write. @@ -299,6 +216,7 @@ def integrate_agents_for_target( agents_dir_created = True target_path.parent.mkdir(parents=True, exist_ok=True) write_text_lf(target_path, rendered) + total_links_resolved += links_resolved files_integrated += 1 target_paths.append(target_path) continue @@ -310,8 +228,25 @@ def integrate_agents_for_target( rel_path = portable_relpath(target_path, project_root) + if mapping.format_id == "codex_agent": + rendered = self._render_codex_agent( + source_file, + resolved_content, + diagnostics=diagnostics, + package_name=package_info.package.name, + ) + else: + rendered = resolved_content + skip, adopted = self._check_adopt_or_skip( - target_path, source_file, rel_path, managed_files, force, diagnostics, target_paths + target_path, + source_file, + rel_path, + managed_files, + force, + diagnostics, + target_paths, + expected_content=rendered, ) if skip: if adopted: @@ -321,19 +256,13 @@ def integrate_agents_for_target( continue if mapping.format_id == "codex_agent": - self._write_codex_agent( - source_file, - target_path, - diagnostics=diagnostics, - package_name=package_info.package.name, - ) - links_resolved = 0 + write_text_lf(target_path, rendered) else: if mapping.format_id == "opencode_agent": self._warn_opencode_frontmatter( source_file, diagnostics, package_info.package.name ) - links_resolved = self.copy_agent(source_file, target_path) + write_text_lf(target_path, rendered) total_links_resolved += links_resolved files_integrated += 1 target_paths.append(target_path) @@ -503,24 +432,20 @@ def _warn_codex_tools_dropped( ) @staticmethod - def _write_codex_agent( + def _render_codex_agent( source: Path, - target: Path, + content: str, *, diagnostics: DiagnosticCollector | None = None, package_name: str = "", - ) -> None: - """Transform an ``.agent.md`` file to Codex ``.toml`` format. + ) -> str: + """Transform agent Markdown content to Codex TOML. Parses YAML frontmatter for ``name`` and ``description``, uses the markdown body as ``developer_instructions``. """ - if source.is_symlink(): - raise ValueError(f"Refusing to read symlink source: {source}") import toml as _toml - content = source.read_text(encoding="utf-8") - name = source.stem if name.endswith(".agent"): name = name[: -len(".agent")] @@ -564,7 +489,27 @@ def _write_codex_agent( "description": description, "developer_instructions": body.strip(), } - write_text_lf(target, _toml.dumps(doc)) + return _toml.dumps(doc) + + @staticmethod + def _write_codex_agent( + source: Path, + target: Path, + *, + diagnostics: DiagnosticCollector | None = None, + package_name: str = "", + ) -> None: + """Read and transform an agent Markdown file to Codex TOML.""" + if source.is_symlink(): + raise ValueError(f"Refusing to read symlink source: {source}") + content = source.read_text(encoding="utf-8") + rendered = AgentIntegrator._render_codex_agent( + source, + content, + diagnostics=diagnostics, + package_name=package_name, + ) + write_text_lf(target, rendered) # ------------------------------------------------------------------ # Kiro agent transformer (MD -> filtered MD) @@ -602,6 +547,7 @@ def _kiro_agent_relpath(source_file: Path, package_path: Path) -> str: def _preflight_render_kiro_agent( source: Path, *, + content: str | None = None, diagnostics=None, package_name: str = "", ) -> tuple[str | None, bool]: @@ -623,7 +569,8 @@ def _preflight_render_kiro_agent( if source.is_symlink(): raise ValueError(f"Refusing to read symlink source: {source}") - content = source.read_text(encoding="utf-8") + if content is None: + content = source.read_text(encoding="utf-8") body = content out_fm: dict = {} @@ -757,11 +704,7 @@ def integrate_package_agents( copilot = KNOWN_TARGETS["copilot"] self.init_link_resolver(package_info, project_root) - agent_files = self.prepare_agent_files( - package_info.install_path, - package_info.package.name, - diagnostics, - ) + agent_files = self.find_agent_files(package_info.install_path) if not agent_files: return IntegrationResult(0, 0, 0, []) diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index fc6ad18e2f..72d5a97975 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -396,6 +396,7 @@ def _check_adopt_or_skip( force: bool, diagnostics, target_paths: list, + expected_content: str | None = None, ) -> tuple[bool, bool]: """Check whether *target_path* should be adopted or skipped. @@ -431,9 +432,20 @@ def _check_adopt_or_skip( is ``True`` only when the existing file already matched the deployed content and has been silently adopted. """ - if self.is_content_identical_to_source( + identical = False + if expected_content is not None and target_path.is_file() and not target_path.is_symlink(): + try: + expected = expected_content + if self._LF_NORMALIZED_DEPLOY: + expected = normalize_crlf_to_lf(expected) + identical = target_path.read_bytes() == expected.encode("utf-8") + except OSError: + identical = False + elif self.is_content_identical_to_source( target_path, source_file, lf_normalized_deploy=self._LF_NORMALIZED_DEPLOY ): + identical = True + if identical: target_paths.append(target_path) return True, True if self.check_collision( @@ -771,6 +783,13 @@ def init_link_resolver(self, package_info, project_root: Path) -> None: self.link_resolver.deployment_package_root = Path( package_info.deployment_package_root or install_path ) + from apm_cli.models.validation import PackageType + + if ( + getattr(package_info, "package_type", None) + is PackageType.MARKETPLACE_PLUGIN + ): + self.link_resolver.claude_plugin_root = install_path except Exception: self.link_resolver = None diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index 1247c55ef8..f144e2f470 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -6,7 +6,7 @@ every guard executes exactly once per run. Names prove nothing about teeth: a rule whose body was gutted still registers its guard ID and still runs. -This file supplies the missing half of that contract. For each of the 56 +This file supplies the missing half of that contract. For each of the 55 registered owner guards it pins one minimal, meaningful source mutation -- a surgical edit that kills a load-bearing sub-condition of the owning decision -- and asserts the one rule that owns that guard reports a real `Violation`. @@ -203,14 +203,6 @@ class MutationCase: new="include_scoped_in_user_root_context: bool = True", intent="TargetProfile flips the user-root scoped-instruction eligibility default.", ), - MutationCase( - guard_id="install-deployment-agent-source-inventory", - rule_id="install-deployment-agent-source-inventory", - path="src/apm_cli/integration/agent_integrator.py", - old=" def prepare_agent_files(", - new=" def prepare_agent_files_disabled(", - intent="AgentIntegrator loses its canonical package-level inventory entry point.", - ), MutationCase( guard_id="install-deployment-audit-replay", rule_id="install-deployment-audit-replay", diff --git a/tests/integration/test_install_services_orchestration.py b/tests/integration/test_install_services_orchestration.py index 5097492190..d43f8e104e 100644 --- a/tests/integration/test_install_services_orchestration.py +++ b/tests/integration/test_install_services_orchestration.py @@ -418,33 +418,6 @@ def test_dispatch_calls_agent_integrator(self, tmp_path: Path) -> None: integrators["agent_integrator"].integrate_agents_for_target.assert_called_once() assert result["agents"] == 1 - def test_agent_files_are_prepared_once_for_multiple_targets(self, tmp_path: Path) -> None: - targets = [ - make_target(name="copilot", primitives={"agents": make_mapping(subdir="agents")}), - make_target(name="claude", primitives={"agents": make_mapping(subdir="agents")}), - ] - entry = make_dispatch_entry( - integrate_method="integrate_agents_for_target", - counter_key="agents", - ) - - _, integrators, diagnostics, _ = invoke_integrate( - tmp_path, - targets=targets, - dispatch_table={"agents": entry}, - integrator_results={"agents": make_integration_result(files_integrated=1)}, - ) - - agent_integrator = integrators["agent_integrator"] - agent_integrator.prepare_agent_files.assert_called_once() - assert agent_integrator.integrate_agents_for_target.call_count == 2 - prepared = agent_integrator.prepare_agent_files.return_value - assert all( - call.kwargs["agent_files"] is prepared - for call in agent_integrator.integrate_agents_for_target.call_args_list - ) - assert agent_integrator.prepare_agent_files.call_args.args[2] is diagnostics - def test_instruction_cursor_rules_use_rule_label(self, tmp_path: Path) -> None: target = make_target( primitives={ diff --git a/tests/integration/test_integrators_validation_rules.py b/tests/integration/test_integrators_validation_rules.py index e2a542ae9f..f643cf1fce 100644 --- a/tests/integration/test_integrators_validation_rules.py +++ b/tests/integration/test_integrators_validation_rules.py @@ -453,17 +453,13 @@ def test_finds_agent_md_in_apm_agents_subdir(self, tmp_path: Path) -> None: assert any(f.name == "reviewer.agent.md" for f in files) def test_finds_plain_md_in_apm_agents_subdir(self, tmp_path: Path) -> None: - """Plain .md files need agent frontmatter.""" + """Plain .md files in .apm/agents/ are also included.""" apm_agents = tmp_path / ".apm" / "agents" apm_agents.mkdir(parents=True) - (apm_agents / "helper.md").write_text( - "---\nname: helper\ndescription: Helps with tasks\n---\n# Helper" - ) - (apm_agents / "missing-description.md").write_text("---\nname: helper\n---\n# Helper") + (apm_agents / "helper.md").write_text("# Helper") integrator = AgentIntegrator() files = integrator.find_agent_files(tmp_path) - names = {file.name for file in files} - assert names == {"helper.md"} + assert any(f.name == "helper.md" for f in files) def test_finds_chatmode_in_apm_chatmodes_subdir(self, tmp_path: Path) -> None: apm_chatmodes = tmp_path / ".apm" / "agents" diff --git a/tests/integration/test_marketplace_plugin_integration.py b/tests/integration/test_marketplace_plugin_integration.py index 81889f53e2..fbff24ee8d 100644 --- a/tests/integration/test_marketplace_plugin_integration.py +++ b/tests/integration/test_marketplace_plugin_integration.py @@ -9,24 +9,17 @@ import json import shutil -from dataclasses import replace from datetime import datetime from pathlib import Path -from types import SimpleNamespace -from unittest.mock import MagicMock, patch import pytest from click.testing import CliRunner from apm_cli.commands.install import install -from apm_cli.deps.plugin_parser import _map_plugin_artifacts -from apm_cli.install.services import IntegratorBundle, integrate_package_primitives from apm_cli.integration.agent_integrator import AgentIntegrator from apm_cli.integration.command_integrator import CommandIntegrator from apm_cli.integration.prompt_integrator import PromptIntegrator from apm_cli.integration.skill_integrator import SkillIntegrator -from apm_cli.integration.targets import KNOWN_TARGETS -from apm_cli.utils.diagnostics import CATEGORY_WARNING, DiagnosticCollector from src.apm_cli.models.apm_package import ( APMPackage, GitReferenceType, @@ -243,87 +236,6 @@ def test_plugin_detection_and_structure_mapping(self, tmp_path): "Command should be mapped to prompts" ) - def test_declared_agent_directory_flattens_agents_and_reports_resources(self, tmp_path): - """Legacy projection stays discoverable and reports unsupported resources.""" - plugin_dir = tmp_path / "plugin" - agent_dir = plugin_dir / "agents" / "my-agent" - (agent_dir / "scripts").mkdir(parents=True) - (agent_dir / "my-agent.md").write_text( - "---\nname: my-agent\ndescription: Test agent\n---\n# Agent\n" - ) - (agent_dir / "scripts" / "helper.py").write_text("print('helper')\n") - apm_dir = plugin_dir / ".apm" - apm_dir.mkdir() - _map_plugin_artifacts( - plugin_dir, - apm_dir, - manifest={"agents": ["./agents/my-agent"]}, - ) - - package = APMPackage(name="test-pkg", version="1.0.0", package_path=plugin_dir) - package_info = PackageInfo( - package=package, - install_path=plugin_dir, - resolved_reference=ResolvedReference( - original_ref="main", - ref_type=GitReferenceType.BRANCH, - resolved_commit="abc123", - ref_name="main", - ), - installed_at=datetime.now().isoformat(), - ) - targets = [ - replace( - KNOWN_TARGETS[target_name], - primitives={"agents": KNOWN_TARGETS[target_name].primitives["agents"]}, - ) - for target_name in ("copilot", "claude") - ] - (tmp_path / ".claude").mkdir() - diagnostics = DiagnosticCollector() - integrator = AgentIntegrator() - hook_integrator = MagicMock() - hook_integrator.reconcile_package_target_restriction = None - skill_integrator = MagicMock() - skill_integrator.integrate_package_skill.return_value = SimpleNamespace( - target_paths=[], - skill_created=False, - sub_skills_promoted=0, - bin_deployed=0, - bin_skipped_reason=None, - ) - - with patch.object( - integrator, - "prepare_agent_files", - wraps=integrator.prepare_agent_files, - ) as prepare_agent_files: - result = integrate_package_primitives( - package_info, - tmp_path, - targets=targets, - integrators=IntegratorBundle( - prompt=MagicMock(), - agent=integrator, - skill=skill_integrator, - instruction=MagicMock(), - command=MagicMock(), - hook=hook_integrator, - ), - force=False, - managed_files=set(), - diagnostics=diagnostics, - package_name=package.name, - ) - - prepare_agent_files.assert_called_once() - assert (tmp_path / ".github/agents/my-agent.agent.md").is_file() - assert (tmp_path / ".claude/agents/my-agent.md").is_file() - assert result["agents"] == 2 - warnings = diagnostics.by_category()[CATEGORY_WARNING] - assert len(warnings) == 1 - assert warnings[0].detail == ".apm/agents/scripts/helper.py" - def test_plugin_with_dependencies(self, tmp_path): """Test plugin with dependencies are handled correctly.""" plugin_dir = tmp_path / "plugin-with-deps" diff --git a/tests/unit/compilation/test_link_resolver_phase3.py b/tests/unit/compilation/test_link_resolver_phase3.py index 5575f75da7..2df1842367 100644 --- a/tests/unit/compilation/test_link_resolver_phase3.py +++ b/tests/unit/compilation/test_link_resolver_phase3.py @@ -30,6 +30,7 @@ UnifiedLinkResolver, _remove_frontmatter, _resolve_path, + resolve_claude_plugin_root, resolve_markdown_links, validate_link_targets, ) @@ -88,6 +89,37 @@ def test_dependency_context_registered_with_qualified_name( assert "org/repo:dep.context.md" in resolver.context_registry +def test_resolve_claude_plugin_root_replaces_only_exact_token(base_dir: Path) -> None: + """Plugin-root expansion does not guess at bare or variant paths.""" + plugin_root = base_dir / "apm_modules" / "_local" / "plugin" + content = ( + "${CLAUDE_PLUGIN_ROOT}/scripts/run.py\n" + "${claude_plugin_root}/scripts/lower.py\n" + "scripts/bare.py\n" + ) + + resolved = resolve_claude_plugin_root(content, plugin_root) + + assert resolved == ( + f"{plugin_root.resolve().as_posix()}/scripts/run.py\n" + "${claude_plugin_root}/scripts/lower.py\n" + "scripts/bare.py\n" + ) + + +def test_resolve_claude_plugin_root_uses_yaml_safe_windows_separators() -> None: + """Windows roots do not inject YAML escape sequences into agent content.""" + plugin_root = MagicMock(spec=Path) + plugin_root.resolve.return_value.as_posix.return_value = "C:/Users/dev/plugin" + + resolved = resolve_claude_plugin_root( + 'path: "${CLAUDE_PLUGIN_ROOT}/scripts/run.py"\n', + plugin_root, + ) + + assert resolved == 'path: "C:/Users/dev/plugin/scripts/run.py"\n' + + # --------------------------------------------------------------------------- # _is_external_url # --------------------------------------------------------------------------- diff --git a/tests/unit/install/test_security_scan_scope.py b/tests/unit/install/test_security_scan_scope.py index 2fd9f0c723..f89c874665 100644 --- a/tests/unit/install/test_security_scan_scope.py +++ b/tests/unit/install/test_security_scan_scope.py @@ -99,30 +99,6 @@ def test_source_only_hidden_character_is_not_in_authorized_scan(tmp_path: Path) assert _pre_deploy_security_scan(plan, DiagnosticCollector(), package_name="clean") is True -def test_non_agent_markdown_is_not_in_authorized_agent_scan(tmp_path: Path) -> None: - """Agent admission and pre-deploy scanning share one file vocabulary.""" - agents = tmp_path / ".apm" / "agents" - agents.mkdir(parents=True) - (agents / "reviewer.md").write_text( - "---\nname: reviewer\ndescription: Reviews changes\n---\n# Reviewer\n", - encoding="utf-8", - ) - (agents / "README.md").write_text("source-only \u202e documentation\n", encoding="utf-8") - - plan = DeployableSourcePlan.create( - _package(tmp_path), - [_primitive_target("agents")], - skill_subset=None, - hooks_approved=False, - canvas_approved=False, - skip_bin=True, - ) - verdict = SecurityGate.scan_files(tmp_path, path_filter=plan.includes) - - assert plan.paths == frozenset({".apm/agents/reviewer.md"}) - assert verdict.should_block is False - - def test_root_skill_plan_preserves_arbitrary_files_but_excludes_internal_content( tmp_path: Path, ) -> None: diff --git a/tests/unit/integration/test_agent_integrator.py b/tests/unit/integration/test_agent_integrator.py index 872e6a9941..31153b03e9 100644 --- a/tests/unit/integration/test_agent_integrator.py +++ b/tests/unit/integration/test_agent_integrator.py @@ -3,11 +3,16 @@ import tempfile from datetime import datetime from pathlib import Path -from unittest.mock import Mock, patch +from unittest.mock import Mock -from apm_cli.install.deployable_source_plan import DeployableSourcePlan from apm_cli.integration import AgentIntegrator -from apm_cli.models.apm_package import APMPackage, GitReferenceType, PackageInfo, ResolvedReference +from apm_cli.models.apm_package import ( + APMPackage, + GitReferenceType, + PackageInfo, + PackageType, + ResolvedReference, +) from apm_cli.utils.diagnostics import ( CATEGORY_AGENT_LOSSY_COMPILATION, CATEGORY_WARNING, @@ -509,18 +514,15 @@ def test_find_agent_files_ignores_skill_files(self): assert "SKILL.md" not in found_names assert "skill.md" not in found_names - def test_find_agent_files_requires_frontmatter_for_plain_md(self): - """Plain Markdown needs agent frontmatter to be an agent.""" + def test_find_agent_files_includes_all_md(self): + """All .md files in .apm/agents/ are discovered — the directory + already implies type, so no name-based filtering.""" package_dir = self.project_root / "package" apm_agents = package_dir / ".apm" / "agents" apm_agents.mkdir(parents=True) - (apm_agents / "planner.md").write_text( - "---\nname: planner\ndescription: Plans implementation work\n---\n# Planner agent" - ) - (apm_agents / "coder.md").write_text( - "---\nname: coder\ndescription: Implements approved plans\n---\n# Coder agent" - ) + (apm_agents / "planner.md").write_text("# Planner agent") + (apm_agents / "coder.md").write_text("# Coder agent") (apm_agents / "README.md").write_text("# Docs") (apm_agents / "CHANGELOG.md").write_text("# Changes") (apm_agents / "LICENSE.md").write_text("MIT") @@ -529,19 +531,14 @@ def test_find_agent_files_requires_frontmatter_for_plain_md(self): agents = self.integrator.find_agent_files(package_dir) names = {a.name for a in agents} - assert names == {"planner.md", "coder.md"} - - def test_find_agent_files_accepts_bom_prefixed_plain_agent(self): - """BOM-prefixed Markdown routes through canonical frontmatter parsing.""" - package_dir = self.project_root / "package" - apm_agents = package_dir / ".apm" / "agents" - apm_agents.mkdir(parents=True) - agent = apm_agents / "planner.md" - agent.write_bytes( - b"\xef\xbb\xbf---\nname: planner\ndescription: Plans implementation work\n---\n# Planner\n" - ) - - assert self.integrator.find_agent_files(package_dir) == [agent] + assert names == { + "planner.md", + "coder.md", + "README.md", + "CHANGELOG.md", + "LICENSE.md", + "CONTRIBUTING.md", + } def test_find_agent_files_discovers_nested_subdirectories(self): """find_agent_files uses rglob so agents in subdirs are found.""" @@ -552,9 +549,7 @@ def test_find_agent_files_discovers_nested_subdirectories(self): (apm_agents / "top-level.agent.md").write_text("# Top") (nested / "nested.agent.md").write_text("# Nested agent.md") - (nested / "plain-nested.md").write_text( - "---\nname: plain-nested\ndescription: Nested plain agent\n---\n# Nested plain" - ) + (nested / "plain-nested.md").write_text("# Nested plain") agents = self.integrator.find_agent_files(package_dir) names = {a.name for a in agents} @@ -563,109 +558,76 @@ def test_find_agent_files_discovers_nested_subdirectories(self): assert "nested.agent.md" in names assert "plain-nested.md" in names - def test_nested_agent_source_filters_resources_and_flattens_agent_path(self): - """Nested resources are rejected while legacy agent output stays flat.""" + def test_legacy_plugin_root_resolves_for_plain_codex_and_kiro_targets(self): + """Every target renders agent content through the shared resolver.""" + import toml + from apm_cli.integration.targets import KNOWN_TARGETS - package_dir = self.project_root / "package" - agent_dir = package_dir / ".apm" / "agents" / "my-agent" - (agent_dir / "guides").mkdir(parents=True) - (agent_dir / "scripts").mkdir() - (agent_dir / "my-agent.md").write_text( - "---\nname: my-agent\ndescription: Test agent\n---\nUse scripts/helper.py.\n" + package_dir = self.project_root / "apm_modules" / "_local" / "plugin" + agents_dir = package_dir / ".apm" / "agents" + agents_dir.mkdir(parents=True) + source = agents_dir / "builder.md" + source.write_text( + "---\nname: builder\ndescription: Builds assets\n---\n" + "Run `${CLAUDE_PLUGIN_ROOT}/scripts/build.py`.\n", + encoding="utf-8", ) - (agent_dir / "guides" / "reference-doc.md").write_text("# Reference\n") - (agent_dir / "scripts" / "helper.py").write_text("print('helper')\n") - - package = APMPackage(name="test-pkg", version="1.0.0", package_path=package_dir) + package = APMPackage(name="plugin", version="1.0.0", package_path=package_dir) package_info = PackageInfo( package=package, install_path=package_dir, - resolved_reference=ResolvedReference( - original_ref="main", - ref_type=GitReferenceType.BRANCH, - resolved_commit="abc123", - ref_name="main", - ), - installed_at=datetime.now().isoformat(), + package_type=PackageType.MARKETPLACE_PLUGIN, ) - diagnostics = DiagnosticCollector() - source_plan = DeployableSourcePlan.create( - package_info, - [KNOWN_TARGETS["copilot"]], - skill_subset=None, - hooks_approved=False, - canvas_approved=False, - skip_bin=True, + (self.project_root / ".codex").mkdir() + (self.project_root / ".kiro").mkdir() + + expected_root = str(package_dir.resolve()) + for target_name in ("copilot", "codex", "kiro"): + result = self.integrator.integrate_agents_for_target( + KNOWN_TARGETS[target_name], + package_info, + self.project_root, + ) + assert result.files_integrated == 1 + output = result.target_paths[0].read_text(encoding="utf-8") + if target_name == "codex": + output = toml.loads(output)["developer_instructions"] + assert f"{expected_root}/scripts/build.py" in output + assert "${CLAUDE_PLUGIN_ROOT}" not in output + + adopted = self.integrator.integrate_agents_for_target( + KNOWN_TARGETS[target_name], + package_info, + self.project_root, + ) + assert adopted.files_adopted == 1 + assert adopted.files_skipped == 0 + + def test_plugin_root_token_is_not_resolved_for_apm_packages(self): + """Claude's plugin token remains literal outside legacy plugins.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + package_dir = self.project_root / "package" + agents_dir = package_dir / ".apm" / "agents" + agents_dir.mkdir(parents=True) + (agents_dir / "builder.agent.md").write_text( + "Use ${CLAUDE_PLUGIN_ROOT}/scripts/build.py.\n", + encoding="utf-8", + ) + package_info = PackageInfo( + package=APMPackage(name="package", version="1.0.0", package_path=package_dir), + install_path=package_dir, + package_type=PackageType.APM_PACKAGE, ) result = self.integrator.integrate_agents_for_target( KNOWN_TARGETS["copilot"], package_info, self.project_root, - diagnostics=diagnostics, - source_plan=source_plan, ) - expected = self.project_root / ".github" / "agents" / "my-agent.agent.md" - assert result.target_paths == [expected] - assert expected.is_file() - assert not (self.project_root / ".github" / "agents" / "reference-doc.agent.md").exists() - warnings = diagnostics.by_category().get(CATEGORY_WARNING, []) - assert len(warnings) == 1 - assert warnings[0].detail == ( - ".apm/agents/my-agent/guides/reference-doc.md, .apm/agents/my-agent/scripts/helper.py" - ) - - def test_prepare_agent_files_sanitizes_ignored_resource_diagnostic(self): - """Package-controlled diagnostic fields stay printable ASCII.""" - package_dir = self.project_root / "package" - agent_dir = package_dir / ".apm" / "agents" - agent_dir.mkdir(parents=True) - (agent_dir / "agent.agent.md").write_text("# Agent\n") - (agent_dir / "helper\nscript.py").write_text("print('helper')\n") - diagnostics = DiagnosticCollector() - - files = self.integrator.prepare_agent_files( - package_dir, - "unsafe\npackage", - diagnostics, - ) - - assert [path.name for path in files] == ["agent.agent.md"] - warnings = diagnostics.by_category()[CATEGORY_WARNING] - assert warnings[0].package == "unsafe?package" - assert warnings[0].detail == ".apm/agents/helper?script.py" - assert "Package required runtime resources as a skill bundle" in warnings[0].message - - def test_prepare_agent_files_caps_ignored_resource_detail(self): - """Ignored-resource diagnostics stay bounded for large bundles.""" - package_dir = self.project_root / "package" - agent_dir = package_dir / ".apm" / "agents" - agent_dir.mkdir(parents=True) - for index in range(25): - (agent_dir / f"resource-{index:02}.txt").write_text("resource\n") - diagnostics = DiagnosticCollector() - - self.integrator.prepare_agent_files(package_dir, "large-package", diagnostics) - - warning = diagnostics.by_category()[CATEGORY_WARNING][0] - assert warning.detail.count(".apm/agents/resource-") == 20 - assert warning.detail.endswith(", ... (+5 more)") - - def test_prepare_agent_files_fallback_keeps_paths_out_of_summary(self): - """Fallback warnings stay concise when no verbose collector exists.""" - package_dir = self.project_root / "package" - agent_dir = package_dir / ".apm" / "agents" - agent_dir.mkdir(parents=True) - (agent_dir / "helper.py").write_text("print('helper')\n") - - with patch("apm_cli.integration.agent_integrator._rich_warning") as warning: - self.integrator.prepare_agent_files(package_dir, "test-package") - - message = warning.call_args.args[0] - assert "helper.py" not in message - assert "verbose output" not in message + assert "${CLAUDE_PLUGIN_ROOT}" in result.target_paths[0].read_text(encoding="utf-8") def test_get_target_filename_plain_md(self): """Plain .md files get renamed to .agent.md for .github/agents/.""" diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index b7818bc09b..a8fba7d660 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -606,7 +606,6 @@ def exiting_import( contracts-tooling-frontmatter-yaml contracts-tooling-generation-footer install-deployment-approval-outcome-routing -install-deployment-agent-source-inventory install-deployment-audit-policy-discovery install-deployment-audit-replay install-deployment-base-integrator diff --git a/tests/unit/test_plugin_parser.py b/tests/unit/test_plugin_parser.py index 7eebd18ea8..1be208a0e7 100644 --- a/tests/unit/test_plugin_parser.py +++ b/tests/unit/test_plugin_parser.py @@ -843,31 +843,34 @@ def test_custom_agents_dir_list_flattens_contents(self, tmp_path): "Should not create nested agents/agents/ directory" ) - def test_declared_agent_subdirectory_flattens_into_legacy_staging(self, tmp_path): - """A declared agent directory is flattened for legacy primitive projection.""" + def test_agent_directory_is_recursive_but_exact_file_excludes_support_docs(self, tmp_path): + """Manifest entry shape, not frontmatter, owns agent membership.""" plugin_dir = tmp_path / "plugin" - agent_dir = plugin_dir / "agents" / "my-agent" - (agent_dir / "guides").mkdir(parents=True) - (agent_dir / "scripts").mkdir() - (agent_dir / "my-agent.md").write_text( - "---\nname: my-agent\ndescription: Test agent\n---\nUse scripts/helper.py.\n" + agent_dir = plugin_dir / "agents" / "builder" + guides_dir = agent_dir / "guides" + guides_dir.mkdir(parents=True) + (agent_dir / "builder.md").write_text("# Builder") + (guides_dir / "reference.md").write_text("# Reference") + + recursive_apm = plugin_dir / "recursive" / ".apm" + recursive_apm.mkdir(parents=True) + _map_plugin_artifacts( + plugin_dir, + recursive_apm, + manifest={"agents": ["./agents/builder"]}, ) - (agent_dir / "guides" / "reference-doc.md").write_text("# Reference\n") - (agent_dir / "scripts" / "helper.py").write_text("print('helper')\n") + assert (recursive_apm / "agents" / "builder.md").is_file() + assert (recursive_apm / "agents" / "guides" / "reference.md").is_file() - apm_dir = plugin_dir / ".apm" - apm_dir.mkdir() + exact_apm = plugin_dir / "exact" / ".apm" + exact_apm.mkdir(parents=True) _map_plugin_artifacts( plugin_dir, - apm_dir, - manifest={"agents": ["./agents/my-agent"]}, + exact_apm, + manifest={"agents": ["./agents/builder/builder.md"]}, ) - - staged = apm_dir / "agents" - assert (staged / "my-agent.md").is_file() - assert (staged / "guides" / "reference-doc.md").is_file() - assert (staged / "scripts" / "helper.py").is_file() - assert not (staged / "my-agent").exists() + assert (exact_apm / "agents" / "builder.md").is_file() + assert not (exact_apm / "agents" / "guides").exists() class TestGenerateApmYml: diff --git a/tests/unit/test_surviving_deps_reintegration.py b/tests/unit/test_surviving_deps_reintegration.py index 4da9fa3eb3..823e4ea0dd 100644 --- a/tests/unit/test_surviving_deps_reintegration.py +++ b/tests/unit/test_surviving_deps_reintegration.py @@ -358,8 +358,6 @@ def test_uninstall_reintegration_preserves_user_scope_and_denies_bin_trust( lockfile = LockFile() lockfile.add_dependency(LockedDependency(repo_url="acme/survivor", depth=1)) observed: list[dict] = [] - logger = MagicMock() - logger.verbose = True def _record_integration(*_args, **kwargs): observed.append(kwargs) @@ -373,7 +371,7 @@ def _record_integration(*_args, **kwargs): APMPackage.from_apm_yml(tmp_path / "apm.yml"), tmp_path, set(), - logger, + MagicMock(), lockfile=lockfile, user_scope=True, ) @@ -384,7 +382,6 @@ def _record_integration(*_args, **kwargs): assert observed[0]["scope"] is InstallScope.USER assert observed[0]["trust_bin"] is False assert observed[0]["bin_skip_reason_override"] == "not_retrusted_on_uninstall" - assert observed[0]["diagnostics"].verbose is True def test_hook_reintegration_sanitizes_blocked_dependency_identity( From b83b4b06f43122f80c8833d4755e262d4a94396c Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 1 Sep 2026 11:17:48 +0200 Subject: [PATCH 13/15] fix: stabilize plugin root replay Use the durable deployment package path during drift replay and preserve no-follow adoption reads for rendered output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/integration/base_integrator.py | 8 +++-- .../unit/integration/test_agent_integrator.py | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index 72d5a97975..800522b091 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -433,12 +433,12 @@ def _check_adopt_or_skip( deployed content and has been silently adopted. """ identical = False - if expected_content is not None and target_path.is_file() and not target_path.is_symlink(): + if expected_content is not None and not target_path.is_symlink(): try: expected = expected_content if self._LF_NORMALIZED_DEPLOY: expected = normalize_crlf_to_lf(expected) - identical = target_path.read_bytes() == expected.encode("utf-8") + identical = _read_bytes_no_follow(target_path) == expected.encode("utf-8") except OSError: identical = False elif self.is_content_identical_to_source( @@ -789,7 +789,9 @@ def init_link_resolver(self, package_info, project_root: Path) -> None: getattr(package_info, "package_type", None) is PackageType.MARKETPLACE_PLUGIN ): - self.link_resolver.claude_plugin_root = install_path + self.link_resolver.claude_plugin_root = ( + self.link_resolver.deployment_package_root + ) except Exception: self.link_resolver = None diff --git a/tests/unit/integration/test_agent_integrator.py b/tests/unit/integration/test_agent_integrator.py index 31153b03e9..ed7d5e0635 100644 --- a/tests/unit/integration/test_agent_integrator.py +++ b/tests/unit/integration/test_agent_integrator.py @@ -604,6 +604,35 @@ def test_legacy_plugin_root_resolves_for_plain_codex_and_kiro_targets(self): assert adopted.files_adopted == 1 assert adopted.files_skipped == 0 + def test_plugin_root_uses_stable_deployment_path_during_replay(self): + """Replay output must match the path used by the original installation.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + replay_root = self.project_root / "replay" / "plugin" + agents_dir = replay_root / ".apm" / "agents" + agents_dir.mkdir(parents=True) + (agents_dir / "builder.md").write_text( + "Run ${CLAUDE_PLUGIN_ROOT}/scripts/build.py.\n", + encoding="utf-8", + ) + deployment_root = self.project_root / "apm_modules" / "owner" / "plugin" + package_info = PackageInfo( + package=APMPackage(name="plugin", version="1.0.0", package_path=replay_root), + install_path=replay_root, + deployment_package_root=deployment_root, + package_type=PackageType.MARKETPLACE_PLUGIN, + ) + + result = self.integrator.integrate_agents_for_target( + KNOWN_TARGETS["copilot"], + package_info, + self.project_root, + ) + + output = result.target_paths[0].read_text(encoding="utf-8") + assert f"{deployment_root.resolve().as_posix()}/scripts/build.py" in output + assert replay_root.resolve().as_posix() not in output + def test_plugin_root_token_is_not_resolved_for_apm_packages(self): """Claude's plugin token remains literal outside legacy plugins.""" from apm_cli.integration.targets import KNOWN_TARGETS From 0f8aee36280f3d0ac393ba944fd4471071cdc99a Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 1 Sep 2026 11:23:05 +0200 Subject: [PATCH 14/15] fix: preserve plugin root in drift replay Carry the durable live bundle root separately from scratch projection paths so replayed legacy plugin agents match installed output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../marketplace_package_and_registration.py | 22 +++++++++++++++++++ src/apm_cli/install/drift.py | 9 ++++++-- src/apm_cli/integration/base_integrator.py | 4 ++-- src/apm_cli/models/apm_package.py | 1 + tests/unit/install/test_drift.py | 3 +++ .../unit/integration/test_agent_integrator.py | 1 + 6 files changed, 36 insertions(+), 4 deletions(-) diff --git a/scripts/architecture_linter/checks/marketplace_package_and_registration.py b/scripts/architecture_linter/checks/marketplace_package_and_registration.py index b8b7183ad4..ea22d73b34 100644 --- a/scripts/architecture_linter/checks/marketplace_package_and_registration.py +++ b/scripts/architecture_linter/checks/marketplace_package_and_registration.py @@ -206,6 +206,8 @@ def _scan_normalization_callers( _SKILL_INTEGRATOR = "src/apm_cli/integration/skill_integrator.py" _PLUGIN_ROOT_OWNER = "src/apm_cli/compilation/link_resolver.py" +_BASE_INTEGRATOR = "src/apm_cli/integration/base_integrator.py" +_DRIFT = "src/apm_cli/install/drift.py" _SKILL_SUBSET_LEXICAL = re.compile( @@ -250,6 +252,26 @@ def _check_legacy_skill_membership(provider: FactsProvider) -> tuple[Violation, "plugin_parser must delegate plugin-root expansion to link_resolver", ) ) + findings.extend( + _require_subs( + provider, + inv, + _RID_SKILL, + _BASE_INTEGRATOR, + ("package_info.claude_plugin_root or install_path",), + "integrators must consume the durable Claude plugin root", + ) + ) + findings.extend( + _require_subs( + provider, + inv, + _RID_SKILL, + _DRIFT, + ("package_info.claude_plugin_root = dependency_ref.get_install_path(",), + "drift replay must preserve the live Claude plugin root", + ) + ) integrator_facts, failures = _load(provider, inv, _RID_SKILL, _SKILL_INTEGRATOR, parse=True) if failures: diff --git a/src/apm_cli/install/drift.py b/src/apm_cli/install/drift.py index c6bd857ad9..f5d3c0bf07 100644 --- a/src/apm_cli/install/drift.py +++ b/src/apm_cli/install/drift.py @@ -654,9 +654,14 @@ def run_replay(config: ReplayConfig, logger: CheckLogger) -> Path: if lock_dep.local_path == _SELF_KEY: package_info.root_local_project_root = project_root package_info.deployment_package_root = scratch_root + package_info.claude_plugin_root = project_root else: - package_info.deployment_package_root = ( - lock_dep.to_dependency_ref().get_install_path(scratch_root / "apm_modules") + dependency_ref = lock_dep.to_dependency_ref() + package_info.deployment_package_root = dependency_ref.get_install_path( + scratch_root / "apm_modules" + ) + package_info.claude_plugin_root = dependency_ref.get_install_path( + live_modules_dir ) dep_key = lock_dep.get_unique_key() diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index 800522b091..dcc5d4ece1 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -789,8 +789,8 @@ def init_link_resolver(self, package_info, project_root: Path) -> None: getattr(package_info, "package_type", None) is PackageType.MARKETPLACE_PLUGIN ): - self.link_resolver.claude_plugin_root = ( - self.link_resolver.deployment_package_root + self.link_resolver.claude_plugin_root = Path( + package_info.claude_plugin_root or install_path ) except Exception: self.link_resolver = None diff --git a/src/apm_cli/models/apm_package.py b/src/apm_cli/models/apm_package.py index 0aee77e78b..87b5a172ac 100644 --- a/src/apm_cli/models/apm_package.py +++ b/src/apm_cli/models/apm_package.py @@ -771,6 +771,7 @@ class PackageInfo: package_type: PackageType | None = None # APM_PACKAGE, CLAUDE_SKILL, or HYBRID root_local_project_root: Path | None = None deployment_package_root: Path | None = None # Source root in the deployment output frame + claude_plugin_root: Path | None = None # Durable bundle root emitted into plugin content def get_canonical_dependency_string(self) -> str: """Get the canonical dependency string for this package. diff --git a/tests/unit/install/test_drift.py b/tests/unit/install/test_drift.py index 7971f4114c..77629bd94b 100644 --- a/tests/unit/install/test_drift.py +++ b/tests/unit/install/test_drift.py @@ -745,6 +745,7 @@ def _spy_integrate(*args, **kwargs): { "target_names": sorted(t.name for t in kwargs.get("targets", [])), "dep_target_subset": kwargs.get("dep_target_subset"), + "claude_plugin_root": args[0].claude_plugin_root, } ) return {"deployed_files": []} @@ -773,6 +774,8 @@ def _spy_integrate(*args, **kwargs): assert call["dep_target_subset"] == ["claude"], ( f"dep_target_subset should be ['claude'], got {call['dep_target_subset']}" ) + expected_plugin_root = dep.to_dependency_ref().get_install_path(project_root / "apm_modules") + assert call["claude_plugin_root"] == expected_plugin_root def test_run_replay_threads_locked_skill_subset( diff --git a/tests/unit/integration/test_agent_integrator.py b/tests/unit/integration/test_agent_integrator.py index ed7d5e0635..1e970cc242 100644 --- a/tests/unit/integration/test_agent_integrator.py +++ b/tests/unit/integration/test_agent_integrator.py @@ -620,6 +620,7 @@ def test_plugin_root_uses_stable_deployment_path_during_replay(self): package=APMPackage(name="plugin", version="1.0.0", package_path=replay_root), install_path=replay_root, deployment_package_root=deployment_root, + claude_plugin_root=deployment_root, package_type=PackageType.MARKETPLACE_PLUGIN, ) From 26c5089768357c36c1822e3fde5c0fff069c9145 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 1 Sep 2026 11:31:59 +0200 Subject: [PATCH 15/15] fix: honor drift module budget Keep durable plugin-root replay wiring within the existing install module line budget. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/install/drift.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/apm_cli/install/drift.py b/src/apm_cli/install/drift.py index f5d3c0bf07..0e2d707038 100644 --- a/src/apm_cli/install/drift.py +++ b/src/apm_cli/install/drift.py @@ -567,7 +567,7 @@ def run_replay(config: ReplayConfig, logger: CheckLogger) -> Path: if config.modules_root is not None else project_root / "apm_modules" ) - live_modules_dir = project_root / "apm_modules" + live_root = project_root / "apm_modules" # Honor apm.yml's ``target:`` field so multi-target projects replay # into all governed roots (not just whichever directory happens to @@ -638,7 +638,7 @@ def run_replay(config: ReplayConfig, logger: CheckLogger) -> Path: apm_modules_dir, cache_only=config.cache_only, lockfile=lock, - live_modules_dir=live_modules_dir, + live_modules_dir=live_root, downloader=downloader, registry_resolver=registry_resolver, registries=registries, @@ -660,9 +660,7 @@ def run_replay(config: ReplayConfig, logger: CheckLogger) -> Path: package_info.deployment_package_root = dependency_ref.get_install_path( scratch_root / "apm_modules" ) - package_info.claude_plugin_root = dependency_ref.get_install_path( - live_modules_dir - ) + package_info.claude_plugin_root = dependency_ref.get_install_path(live_root) dep_key = lock_dep.get_unique_key() integrate_package_primitives(