From 8c5a9931547ebefcf04b20a6e77651f5b2f8ed0e Mon Sep 17 00:00:00 2001 From: xuyuanhao Date: Wed, 15 Apr 2026 11:14:36 +0800 Subject: [PATCH 1/3] fix: deploy root project .apm/ primitives without a sub-package stub apm install silently ignored the project's own root-level .apm/ directory. Users with both external dependencies and local rules in root .apm/ were forced to create a dummy ./agent/apm.yml stub and reference it as a local dependency -- unnecessary boilerplate that the tool should handle natively. Root cause: _install_apm_dependencies() had two early-return guards that fired before target detection and integrator setup, so the project root's own .apm/ was never processed. The integration pipeline only ever ran against paths inside apm_modules/. Fix: detect root .apm/ subdirectories before the early returns so setup proceeds. After the dependency download loop, create a synthetic PackageInfo with install_path=project_root and run it through the same _integrate_package_primitives pipeline used for all other packages. Fixes #714 --- src/apm_cli/commands/install.py | 79 ++++++++++++- tests/integration/test_local_install.py | 151 ++++++++++++++++++++++++ 2 files changed, 226 insertions(+), 4 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 09f57c6e..775b7a99 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -791,8 +791,20 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo old_mcp_servers = builtins.set(_existing_lock.mcp_servers) old_mcp_configs = builtins.dict(_existing_lock.mcp_configs) + # Also enter the APM install path when the project root has local .apm/ + # primitives, even if there are no external APM dependencies (#714). + from apm_cli.core.scope import get_deploy_root as _get_deploy_root + _cli_project_root = _get_deploy_root(scope) + _cli_root_apm = _cli_project_root / ".apm" + _LOCAL_PRIM_SUBDIRS = ('instructions', 'agents', 'chatmodes', 'context', 'memory', 'prompts') + _cli_has_root_primitives = ( + _cli_root_apm.exists() and _cli_root_apm.is_dir() and any( + (_cli_root_apm / d).is_dir() for d in _LOCAL_PRIM_SUBDIRS + ) + ) or (_cli_project_root / "SKILL.md").exists() + apm_diagnostics = None - if should_install_apm and has_any_apm_deps: + if should_install_apm and (has_any_apm_deps or _cli_has_root_primitives): if not APM_DEPS_AVAILABLE: logger.error("APM dependency system not available") logger.progress(f"Import error: {_APM_IMPORT_ERROR}") @@ -1160,12 +1172,24 @@ def _install_apm_dependencies( apm_deps = apm_package.get_apm_dependencies() dev_apm_deps = apm_package.get_dev_apm_dependencies() all_apm_deps = apm_deps + dev_apm_deps - if not all_apm_deps: - return InstallResult() project_root = get_deploy_root(scope) apm_dir = get_apm_dir(scope) + # Check whether the project root itself has local .apm/ primitives (#714). + # Users should be able to keep root-level .apm/ rules alongside their apm.yml + # without creating a dummy sub-package stub. + _root_apm_dir = project_root / ".apm" + _LOCAL_PRIM_SUBDIRS = ('instructions', 'agents', 'chatmodes', 'context', 'memory', 'prompts') + _root_has_local_primitives = ( + _root_apm_dir.exists() and _root_apm_dir.is_dir() and any( + (_root_apm_dir / d).is_dir() for d in _LOCAL_PRIM_SUBDIRS + ) + ) or (project_root / "SKILL.md").exists() + + if not all_apm_deps and not _root_has_local_primitives: + return InstallResult() + # T5: Check for existing lockfile - use locked versions for reproducible installs from apm_cli.deps.lockfile import LockFile, get_lockfile_path lockfile_path = get_lockfile_path(apm_dir) @@ -1373,7 +1397,7 @@ def _collect_descendants(node, visited=None): if dep.get_identity() in only_identities ] - if not deps_to_install: + if not deps_to_install and not _root_has_local_primitives: if logger: logger.nothing_to_install() return InstallResult() @@ -2309,6 +2333,53 @@ def _collect_descendants(node, visited=None): # Continue with other packages instead of failing completely continue + # ------------------------------------------------------------------ + # Integrate root project's own .apm/ primitives (#714). + # + # Users should not need a dummy "./agent/apm.yml" stub to get their + # root-level .apm/ rules deployed alongside external dependencies. + # Treat the project root as an implicit local package: any primitives + # found in /.apm/ are integrated after all declared + # dependency packages have been processed. + # ------------------------------------------------------------------ + if _root_has_local_primitives and _targets: + from apm_cli.models.apm_package import PackageInfo as _PackageInfo + _root_pkg_info = _PackageInfo( + package=apm_package, + install_path=project_root, + ) + if logger: + logger.download_complete("", ref_suffix="local") + try: + _root_result = _integrate_package_primitives( + _root_pkg_info, project_root, + targets=_targets, + prompt_integrator=prompt_integrator, + agent_integrator=agent_integrator, + skill_integrator=skill_integrator, + instruction_integrator=instruction_integrator, + command_integrator=command_integrator, + hook_integrator=hook_integrator, + force=force, + managed_files=managed_files, + diagnostics=diagnostics, + package_name="", + logger=logger, + scope=scope, + ) + total_prompts_integrated += _root_result["prompts"] + total_agents_integrated += _root_result["agents"] + total_instructions_integrated += _root_result["instructions"] + total_commands_integrated += _root_result["commands"] + total_hooks_integrated += _root_result["hooks"] + total_links_resolved += _root_result["links_resolved"] + installed_count += 1 + except Exception as e: + diagnostics.error( + f"Failed to integrate root project primitives: {e}", + package="", + ) + # Update .gitignore _update_gitignore_for_apm_modules(logger=logger) diff --git a/tests/integration/test_local_install.py b/tests/integration/test_local_install.py index 31d54db8..36912034 100644 --- a/tests/integration/test_local_install.py +++ b/tests/integration/test_local_install.py @@ -380,6 +380,157 @@ def test_pack_rejects_with_local_deps(self, temp_workspace, apm_command): ) +class TestRootProjectPrimitives: + """Test #714: root project .apm/ integration without a sub-package stub. + + Users should be able to place .apm/ rules directly in their project root + alongside apm.yml without creating a dummy ./agent/apm.yml workaround. + """ + + def _make_project(self, tmp_path, *, apm_deps=None): + """Return a project root with .apm/instructions/ and optional deps.""" + project = tmp_path / "project" + project.mkdir() + + deps_section = {"apm": apm_deps} if apm_deps else {} + (project / "apm.yml").write_text(yaml.dump({ + "name": "my-project", + "version": "1.0.0", + "dependencies": deps_section, + })) + + instructions_dir = project / ".apm" / "instructions" + instructions_dir.mkdir(parents=True) + (instructions_dir / "local-rules.instructions.md").write_text( + "---\napplyTo: '**'\n---\n# Local Rules\nFollow these local rules." + ) + + # Create .claude/rules/ so claude target is auto-detected + (project / ".claude" / "rules").mkdir(parents=True) + return project + + def test_root_apm_primitives_deployed_with_no_deps(self, tmp_path, apm_command): + """root apm.yml with no deps + root .apm/ -> rules deployed. + + Before the fix, apm install returned early with nothing to install + and never deployed the local .apm/ rules. + """ + project = self._make_project(tmp_path) + + result = subprocess.run( + [apm_command, "install"], + cwd=project, + capture_output=True, + text=True, + timeout=60, + ) + combined = result.stdout + result.stderr + assert result.returncode == 0, f"Install failed:\n{combined}" + + deployed = project / ".claude" / "rules" / "local-rules.md" + assert deployed.exists(), ( + f"Root .apm/ rules were NOT deployed to .claude/rules/.\n" + f"Output:\n{combined}" + ) + assert "Local Rules" in deployed.read_text() + + def test_root_apm_primitives_deployed_alongside_external_dep( + self, tmp_path, apm_command + ): + """root apm.yml with external dep + root .apm/ -> both rule sets deployed. + + This is the exact scenario from #714: external dependencies in apm.yml + and local .apm/ rules at the root. Before the fix, only the external + dep's rules were deployed. + """ + ext_pkg = tmp_path / "ext-pkg" + ext_pkg.mkdir() + (ext_pkg / "apm.yml").write_text(yaml.dump({ + "name": "ext-pkg", + "version": "1.0.0", + })) + ext_instr = ext_pkg / ".apm" / "instructions" + ext_instr.mkdir(parents=True) + (ext_instr / "ext-rules.instructions.md").write_text( + "---\napplyTo: '**'\n---\n# External Rules\nFrom external package." + ) + + project = self._make_project(tmp_path, apm_deps=["../ext-pkg"]) + + result = subprocess.run( + [apm_command, "install"], + cwd=project, + capture_output=True, + text=True, + timeout=60, + ) + combined = result.stdout + result.stderr + assert result.returncode == 0, f"Install failed:\n{combined}" + + deployed_names = {f.name for f in (project / ".claude" / "rules").glob("*.md")} + assert "local-rules.md" in deployed_names, ( + f"Root .apm/ rule NOT deployed. Files: {deployed_names}\nOutput:\n{combined}" + ) + assert "ext-rules.md" in deployed_names, ( + f"External dep rule NOT deployed. Files: {deployed_names}\nOutput:\n{combined}" + ) + + def test_workaround_sub_package_still_works(self, tmp_path, apm_command): + """Old ./agent/apm.yml workaround continues to work (regression guard).""" + project = tmp_path / "project" + project.mkdir() + + agent_dir = project / "agent" + agent_dir.mkdir() + (agent_dir / "apm.yml").write_text(yaml.dump({ + "name": "my-project-agent", + "version": "1.0.0", + })) + agent_instr = agent_dir / ".apm" / "instructions" + agent_instr.mkdir(parents=True) + (agent_instr / "agent-rules.instructions.md").write_text( + "---\napplyTo: '**'\n---\n# Agent Rules\nFrom sub-package stub." + ) + + (project / "apm.yml").write_text(yaml.dump({ + "name": "my-project", + "version": "1.0.0", + "dependencies": {"apm": ["./agent"]}, + })) + (project / ".claude" / "rules").mkdir(parents=True) + + result = subprocess.run( + [apm_command, "install"], + cwd=project, + capture_output=True, + text=True, + timeout=60, + ) + combined = result.stdout + result.stderr + assert result.returncode == 0, f"Install failed:\n{combined}" + assert (project / ".claude" / "rules" / "agent-rules.md").exists(), ( + f"Sub-package rules NOT deployed.\nOutput:\n{combined}" + ) + + def test_root_apm_primitives_idempotent(self, tmp_path, apm_command): + """Running apm install twice with root .apm/ is idempotent.""" + project = self._make_project(tmp_path) + + for run in range(2): + result = subprocess.run( + [apm_command, "install"], + cwd=project, + capture_output=True, + text=True, + timeout=60, + ) + assert result.returncode == 0, ( + f"Run {run + 1} failed:\n{result.stdout + result.stderr}" + ) + + assert (project / ".claude" / "rules" / "local-rules.md").exists() + + class TestLocalMixedWithRemote: """Test mixing local and remote dependencies.""" From 35ad6825a739d38df2a8acb26ba05026213e7c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Wed, 15 Apr 2026 23:08:17 +0800 Subject: [PATCH 2/3] refactor: simplify root primitive detection and harden error handling Address review feedback on #715: - Remove _LOCAL_PRIM_SUBDIRS allowlist from both install() and _install_apm_dependencies(). The check now treats .apm/ as a package itself (consistent with how all other packages are handled), letting each integrator decide whether it has content to deploy. This avoids drift if new primitive types are added in the future. - Pass full traceback via diagnostics.error(detail=...) so root integration failures are actionable in the diagnostic report. When root integration is the only action (no external deps), also emit logger.error() so the failure is visible to the user rather than silently swallowed. - Add two integration tests: test_root_apm_hooks_deployed (guards that a hooks-only .apm/ dir does not hit the early-return path) and test_root_skill_md_detected (guards the SKILL.md detection branch). All 6 TestRootProjectPrimitives tests pass. --- src/apm_cli/commands/install.py | 18 +++--- tests/integration/test_local_install.py | 73 +++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index a7bf380d..1c6aa1f4 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -802,11 +802,8 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo from apm_cli.core.scope import get_deploy_root as _get_deploy_root _cli_project_root = _get_deploy_root(scope) _cli_root_apm = _cli_project_root / ".apm" - _LOCAL_PRIM_SUBDIRS = ('instructions', 'agents', 'chatmodes', 'context', 'memory', 'prompts') _cli_has_root_primitives = ( - _cli_root_apm.exists() and _cli_root_apm.is_dir() and any( - (_cli_root_apm / d).is_dir() for d in _LOCAL_PRIM_SUBDIRS - ) + _cli_root_apm.exists() and _cli_root_apm.is_dir() ) or (_cli_project_root / "SKILL.md").exists() apm_diagnostics = None @@ -1420,11 +1417,8 @@ def _install_apm_dependencies( # Users should be able to keep root-level .apm/ rules alongside their apm.yml # without creating a dummy sub-package stub. _root_apm_dir = project_root / ".apm" - _LOCAL_PRIM_SUBDIRS = ('instructions', 'agents', 'chatmodes', 'context', 'memory', 'prompts') _root_has_local_primitives = ( - _root_apm_dir.exists() and _root_apm_dir.is_dir() and any( - (_root_apm_dir / d).is_dir() for d in _LOCAL_PRIM_SUBDIRS - ) + _root_apm_dir.exists() and _root_apm_dir.is_dir() ) or (project_root / "SKILL.md").exists() if not all_apm_deps and not _root_has_local_primitives: @@ -2615,10 +2609,18 @@ def _collect_descendants(node, visited=None): total_links_resolved += _root_result["links_resolved"] installed_count += 1 except Exception as e: + import traceback as _tb diagnostics.error( f"Failed to integrate root project primitives: {e}", package="", + detail=_tb.format_exc(), ) + # When root integration is the *only* action (no external deps), + # a failure means nothing was deployed — surface it clearly. + if not all_apm_deps and logger: + logger.error( + f"Root project primitives could not be integrated: {e}" + ) # Update .gitignore _update_gitignore_for_apm_modules(logger=logger) diff --git a/tests/integration/test_local_install.py b/tests/integration/test_local_install.py index 36912034..98948322 100644 --- a/tests/integration/test_local_install.py +++ b/tests/integration/test_local_install.py @@ -530,6 +530,79 @@ def test_root_apm_primitives_idempotent(self, tmp_path, apm_command): assert (project / ".claude" / "rules" / "local-rules.md").exists() + def test_root_apm_hooks_deployed(self, tmp_path, apm_command): + """root .apm/hooks/ is detected and integrated (not just instructions). + + Guards the _ROOT_PRIM_SUBDIRS list: a project that only has .apm/hooks/ + must still enter the integration path and not hit the early-return guard. + """ + project = tmp_path / "project" + project.mkdir() + + (project / "apm.yml").write_text(yaml.dump({ + "name": "my-project", + "version": "1.0.0", + })) + + hooks_dir = project / ".apm" / "hooks" + hooks_dir.mkdir(parents=True) + (hooks_dir / "on-save.json").write_text( + '{"hooks": {"PostToolUse": [{"matcher": "Write", "hooks": [{"type": "command", "command": "echo saved"}]}]}}' + ) + + # Create .claude/ so claude target is auto-detected + (project / ".claude").mkdir(parents=True) + + result = subprocess.run( + [apm_command, "install"], + cwd=project, + capture_output=True, + text=True, + timeout=60, + ) + combined = result.stdout + result.stderr + assert result.returncode == 0, f"Install failed:\n{combined}" + # The hook integrator merges into settings.json; confirm it was created + # or that install did not silently early-return (exit 0 with no output). + assert "nothing to install" not in combined.lower(), ( + f"Install returned 'nothing to install' — hooks detection guard may " + f"have triggered early return.\nOutput:\n{combined}" + ) + + def test_root_skill_md_detected(self, tmp_path, apm_command): + """A root SKILL.md alone triggers the integration path. + + Guards the (project_root / "SKILL.md").exists() branch in the + root-primitive detection logic. + """ + project = tmp_path / "project" + project.mkdir() + + (project / "apm.yml").write_text(yaml.dump({ + "name": "my-project", + "version": "1.0.0", + })) + (project / "SKILL.md").write_text( + "# My Skill\nThis skill does something useful." + ) + + # Create .claude/ so claude target is auto-detected + (project / ".claude").mkdir(parents=True) + + result = subprocess.run( + [apm_command, "install"], + cwd=project, + capture_output=True, + text=True, + timeout=60, + ) + combined = result.stdout + result.stderr + assert result.returncode == 0, f"Install failed:\n{combined}" + assert "nothing to install" not in combined.lower(), ( + f"Install returned 'nothing to install' — SKILL.md detection may " + f"have been skipped.\nOutput:\n{combined}" + ) + class TestLocalMixedWithRemote: """Test mixing local and remote dependencies.""" From 383a6929f00a9dc7c3fde37ac0bd607a4ba301e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Thu, 16 Apr 2026 12:29:46 +0800 Subject: [PATCH 3/3] refactor: extract root-primitive detection to helper, drop SKILL.md trigger Address reviewer feedback: - Extract the duplicated .apm/ existence check into a single helper function _project_has_root_primitives(), called from both install() and _install_apm_dependencies(). Eliminates the drift risk flagged in review. - Remove the (project_root / "SKILL.md").exists() branch from the detection guard. Triggering a full primitive integration pass solely because a SKILL.md exists at the project root is confusing UX and was not part of the original bug (#714). Root .apm/ directory presence remains the sole trigger. --- src/apm_cli/commands/install.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 1c6aa1f4..60c68dbc 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -57,6 +57,23 @@ _APM_IMPORT_ERROR = str(e) +# --------------------------------------------------------------------------- +# Root primitive detection helper +# --------------------------------------------------------------------------- + +def _project_has_root_primitives(project_root) -> bool: + """Return True when *project_root* has a .apm/ directory of its own. + + Used to decide whether ``apm install`` should enter the integration + pipeline even when no external APM dependencies are declared (#714). + The integrators themselves determine whether the directory contains + anything actionable, so we only check for the directory's existence. + """ + from pathlib import Path as _Path + root = _Path(project_root) + return (root / ".apm").is_dir() + + # --------------------------------------------------------------------------- # Validation helpers # --------------------------------------------------------------------------- @@ -801,13 +818,9 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo # primitives, even if there are no external APM dependencies (#714). from apm_cli.core.scope import get_deploy_root as _get_deploy_root _cli_project_root = _get_deploy_root(scope) - _cli_root_apm = _cli_project_root / ".apm" - _cli_has_root_primitives = ( - _cli_root_apm.exists() and _cli_root_apm.is_dir() - ) or (_cli_project_root / "SKILL.md").exists() apm_diagnostics = None - if should_install_apm and (has_any_apm_deps or _cli_has_root_primitives): + if should_install_apm and (has_any_apm_deps or _project_has_root_primitives(_cli_project_root)): if not APM_DEPS_AVAILABLE: logger.error("APM dependency system not available") logger.progress(f"Import error: {_APM_IMPORT_ERROR}") @@ -1416,10 +1429,7 @@ def _install_apm_dependencies( # Check whether the project root itself has local .apm/ primitives (#714). # Users should be able to keep root-level .apm/ rules alongside their apm.yml # without creating a dummy sub-package stub. - _root_apm_dir = project_root / ".apm" - _root_has_local_primitives = ( - _root_apm_dir.exists() and _root_apm_dir.is_dir() - ) or (project_root / "SKILL.md").exists() + _root_has_local_primitives = _project_has_root_primitives(project_root) if not all_apm_deps and not _root_has_local_primitives: return InstallResult()