Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 77 additions & 4 deletions src/apm_cli/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -797,8 +797,17 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo
old_mcp_configs = builtins.dict(_existing_lock.mcp_configs)
old_local_deployed = builtins.list(_existing_lock.local_deployed_files)

# 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"
_cli_has_root_primitives = (
_cli_root_apm.exists() and _cli_root_apm.is_dir()
) or (_cli_project_root / "SKILL.md").exists()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we be processing it in case the project contains a Skill.md in root? I think that would be confusing UX


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}")
Expand Down Expand Up @@ -1400,12 +1409,21 @@ 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"
_root_has_local_primitives = (
_root_apm_dir.exists() and _root_apm_dir.is_dir()
) or (project_root / "SKILL.md").exists()
Comment thread
edenfunf marked this conversation as resolved.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second time the check appears in the file, seems error prone. Could extract to function.


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)
Expand Down Expand Up @@ -1613,7 +1631,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()
Expand Down Expand Up @@ -2549,6 +2567,61 @@ 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 <project_root>/.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("<project root>", 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="<root>",
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:
import traceback as _tb
diagnostics.error(
f"Failed to integrate root project primitives: {e}",
package="<root>",
detail=_tb.format_exc(),
)
Comment thread
edenfunf marked this conversation as resolved.
# 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)

Expand Down
224 changes: 224 additions & 0 deletions tests/integration/test_local_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,230 @@ 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.
"""
Comment thread
edenfunf marked this conversation as resolved.

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()

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."""

Expand Down