diff --git a/src/apm_cli/install/phases/finalize.py b/src/apm_cli/install/phases/finalize.py index a0db5ab03..8275512e9 100644 --- a/src/apm_cli/install/phases/finalize.py +++ b/src/apm_cli/install/phases/finalize.py @@ -28,6 +28,82 @@ ) +def _has_dep_instruction_files(ctx: InstallContext) -> bool: + """Return True if any installed dep directory contains ``.instructions.md`` files. + + Scans ``apm_modules/`` for ``*.instructions.md`` files under any + ``.apm/instructions/`` subdirectory. Kept deliberately lightweight -- + imports are lazy to avoid pulling the discovery package into the hot + install path. + """ + apm_modules = (ctx.apm_modules_dir or (ctx.project_root / "apm_modules")).resolve() + if not apm_modules.is_dir(): + return False + + for candidate in apm_modules.rglob("*.instructions.md"): + # Only count files that live inside a ``.apm/instructions/`` subtree, + # which is where ``_copy_local_package`` and the bundle staging logic + # place compile-only instruction files. + parts = candidate.parts + try: + idx = parts.index(".apm") + except ValueError: + continue + if idx + 1 < len(parts) and parts[idx + 1] == "instructions": + return True + + return False + + +def _hint_project_compile_needed(ctx: InstallContext) -> None: + """Hint to run ``apm compile`` after installing packages with compile-only instructions. + + Fires on project-scope installs (not global) when BOTH conditions hold: + + 1. At least one active target stores instructions via the compile pipeline + (``compile_family`` is in :data:`_ROOT_CONTEXT_ONLY_FAMILIES`, e.g. gemini, + agents, claude) -- i.e. instructions are NOT written to a per-file rules + directory during ``apm install``. + 2. At least one installed dependency directory contains ``.instructions.md`` + files under its ``.apm/instructions/`` subtree. + + No file is written. Compilation stays explicit: the user runs + ``apm compile`` to materialise AGENTS.md / GEMINI.md / CLAUDE.md. + """ + if ctx.dry_run: + return + if ctx.installed_count == 0: + return + + target_names: list[str] = [] + seen: set[str] = set() + for target in ctx.targets: + scoped = target.for_scope(user_scope=False) + if scoped is None: + continue + if scoped.name.lower() in _ROOT_CONTEXT_HINT_EXCLUDED_TARGETS: + continue + if scoped.compile_family not in _ROOT_CONTEXT_ONLY_FAMILIES: + continue + if scoped.name not in seen: + seen.add(scoped.name) + target_names.append(scoped.name) + + if not target_names: + return + + if not _has_dep_instruction_files(ctx): + return + + if ctx.logger: + targets = ", ".join(target_names) + message = ( + f"Instructions installed for {targets}. " + "Run 'apm compile' to update root context files." + ) + ctx.logger.info(message, symbol="info") + + def _hint_global_root_context(ctx: InstallContext) -> None: """Print a one-line hint pointing at ``apm compile -g`` after ``install -g``. @@ -143,14 +219,15 @@ def run(ctx: InstallContext) -> InstallResult: f"{ctx.unpinned_count} {noun} unpinned -- add #tag or #sha to prevent drift" ) - # User-scope post-install: when global instructions land on a - # root-context-only target, print a one-line hint pointing at - # ``apm compile -g``. No context file is written on install -- - # compilation stays explicit. + # Post-install hints: remind the user to run ``apm compile`` when + # instructions land in deps but are not written to a per-file rules + # directory (compile-only targets such as gemini, agents, claude). from apm_cli.core.scope import InstallScope if ctx.scope is InstallScope.USER: _hint_global_root_context(ctx) + elif ctx.scope is InstallScope.PROJECT: + _hint_project_compile_needed(ctx) return InstallResult( ctx.installed_count, diff --git a/tests/unit/install/phases/test_finalize_user_compile.py b/tests/unit/install/phases/test_finalize_user_compile.py index 72d64f79c..7fa47aaa4 100644 --- a/tests/unit/install/phases/test_finalize_user_compile.py +++ b/tests/unit/install/phases/test_finalize_user_compile.py @@ -1,13 +1,13 @@ -"""Unit tests for finalize.py install-time global-instructions hint. +"""Unit tests for finalize.py install-time compile-hint hooks. -Covers _hint_global_root_context and its integration in run(): +Covers _hint_global_root_context, _hint_project_compile_needed, and run(): * hint fires when global instructions land on a root-context-only target * hint suppressed when no global instructions were installed * hint suppressed when only directory-native targets are active * hint suppressed on dry-run * hint writes NO file (read-only) -* run(): does NOT hint for PROJECT scope +* run(): calls _hint_project_compile_needed for PROJECT scope (regression: #2057) * run(): DOES hint for USER scope """ @@ -299,19 +299,24 @@ def test_hint_writes_no_file(self): class TestFinalizeRunIntegration: """Tests for run() function's integration of the hint hook.""" - def test_project_scope_no_hint(self): - """When ctx.scope is PROJECT, hint hook is NOT called.""" + def test_project_scope_global_hint_not_called(self): + """When ctx.scope is PROJECT, _hint_global_root_context is NOT called.""" from apm_cli.core.scope import InstallScope from apm_cli.install.phases.finalize import run ctx = _make_install_context(scope=InstallScope.PROJECT) - with patch( - "apm_cli.install.phases.finalize._hint_global_root_context", - ) as mock_hint: + with ( + patch( + "apm_cli.install.phases.finalize._hint_global_root_context", + ) as mock_global_hint, + patch( + "apm_cli.install.phases.finalize._hint_project_compile_needed", + ), + ): run(ctx) - mock_hint.assert_not_called() + mock_global_hint.assert_not_called() def test_user_scope_hint_called(self): """When ctx.scope is USER, hint hook IS called with the context.""" @@ -329,17 +334,23 @@ def test_user_scope_hint_called(self): assert result is not None def test_none_scope_no_hint(self): - """When ctx.scope is None, hint hook is NOT called.""" + """When ctx.scope is None, neither hint hook is called.""" from apm_cli.install.phases.finalize import run ctx = _make_install_context(scope=None) - with patch( - "apm_cli.install.phases.finalize._hint_global_root_context", - ) as mock_hint: + with ( + patch( + "apm_cli.install.phases.finalize._hint_global_root_context", + ) as mock_hint, + patch( + "apm_cli.install.phases.finalize._hint_project_compile_needed", + ) as mock_project_hint, + ): run(ctx) mock_hint.assert_not_called() + mock_project_hint.assert_not_called() def test_run_returns_install_result(self): """run() returns an InstallResult object.""" @@ -356,3 +367,162 @@ def test_run_returns_install_result(self): assert isinstance(result, InstallResult) assert result.installed_count == 1 + + +# --------------------------------------------------------------------------- +# _hint_project_compile_needed tests (regression: issue #2057) +# --------------------------------------------------------------------------- + + +class TestHintProjectCompileNeeded: + """Tests for _hint_project_compile_needed() -- the project-scope compile hint. + + Regression guard: before the fix for #2057, installing a local Gemini package + produced no hint. The user had to know independently that ``apm compile`` + was required; GEMINI.md never received the dep's instructions. + """ + + def test_hint_fires_for_compile_only_target_with_instruction_files(self, tmp_path): + """Gemini target + dep has instruction files -> hint is emitted.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.finalize import _hint_project_compile_needed + + # Plant a dep instruction file in apm_modules + instr = tmp_path / "apm_modules" / "_local" / "mypkg" / ".apm" / "instructions" + instr.mkdir(parents=True) + (instr / "rules.instructions.md").write_text("# Rules\n") + + ctx = _make_install_context( + scope=InstallScope.PROJECT, + targets=[_make_target("Gemini CLI", "gemini")], + ) + ctx.apm_modules_dir = tmp_path / "apm_modules" + ctx.project_root = tmp_path + + _hint_project_compile_needed(ctx) + + ctx.logger.info.assert_called_once() + message = ctx.logger.info.call_args.args[0] + assert "apm compile" in message + assert "Gemini CLI" in message + + def test_hint_not_fired_for_copilot_target(self, tmp_path): + """Copilot target (native per-file rules) -> no hint.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.finalize import _hint_project_compile_needed + + instr = tmp_path / "apm_modules" / "_local" / "pkg" / ".apm" / "instructions" + instr.mkdir(parents=True) + (instr / "rules.instructions.md").write_text("# Rules\n") + + ctx = _make_install_context( + scope=InstallScope.PROJECT, + targets=[_make_target("GitHub Copilot", "copilot")], + ) + ctx.apm_modules_dir = tmp_path / "apm_modules" + ctx.project_root = tmp_path + + _hint_project_compile_needed(ctx) + + ctx.logger.info.assert_not_called() + + def test_hint_not_fired_when_no_instruction_files(self, tmp_path): + """Gemini target but no dep instruction files -> no hint.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.finalize import _hint_project_compile_needed + + (tmp_path / "apm_modules").mkdir() + + ctx = _make_install_context( + scope=InstallScope.PROJECT, + targets=[_make_target("Gemini CLI", "gemini")], + ) + ctx.apm_modules_dir = tmp_path / "apm_modules" + ctx.project_root = tmp_path + + _hint_project_compile_needed(ctx) + + ctx.logger.info.assert_not_called() + + def test_hint_not_fired_on_dry_run(self, tmp_path): + """Dry-run installs do not emit the hint.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.finalize import _hint_project_compile_needed + + instr = tmp_path / "apm_modules" / "_local" / "pkg" / ".apm" / "instructions" + instr.mkdir(parents=True) + (instr / "rules.instructions.md").write_text("# Rules\n") + + ctx = _make_install_context( + scope=InstallScope.PROJECT, + targets=[_make_target("Gemini CLI", "gemini")], + dry_run=True, + ) + ctx.apm_modules_dir = tmp_path / "apm_modules" + ctx.project_root = tmp_path + + _hint_project_compile_needed(ctx) + + ctx.logger.info.assert_not_called() + + def test_hint_not_fired_when_nothing_installed(self, tmp_path): + """installed_count == 0 -> no hint (no-op install).""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.finalize import _hint_project_compile_needed + + instr = tmp_path / "apm_modules" / "_local" / "pkg" / ".apm" / "instructions" + instr.mkdir(parents=True) + (instr / "rules.instructions.md").write_text("# Rules\n") + + ctx = _make_install_context( + scope=InstallScope.PROJECT, + targets=[_make_target("Gemini CLI", "gemini")], + ) + ctx.installed_count = 0 + ctx.apm_modules_dir = tmp_path / "apm_modules" + ctx.project_root = tmp_path + + _hint_project_compile_needed(ctx) + + ctx.logger.info.assert_not_called() + + def test_hint_not_fired_for_excluded_target_name(self, tmp_path): + """Targets in the exclusion set are skipped even if family matches.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.finalize import _hint_project_compile_needed + + instr = tmp_path / "apm_modules" / "_local" / "pkg" / ".apm" / "instructions" + instr.mkdir(parents=True) + (instr / "rules.instructions.md").write_text("# Rules\n") + + # "cursor" is in _ROOT_CONTEXT_HINT_EXCLUDED_TARGETS + ctx = _make_install_context( + scope=InstallScope.PROJECT, + targets=[_make_target("cursor", "agents")], + ) + ctx.apm_modules_dir = tmp_path / "apm_modules" + ctx.project_root = tmp_path + + _hint_project_compile_needed(ctx) + + ctx.logger.info.assert_not_called() + + def test_run_calls_project_hint_for_project_scope(self): + """run() calls _hint_project_compile_needed (not global hint) for PROJECT scope.""" + from apm_cli.core.scope import InstallScope + from apm_cli.install.phases.finalize import run + + ctx = _make_install_context(scope=InstallScope.PROJECT) + + with ( + patch( + "apm_cli.install.phases.finalize._hint_project_compile_needed", + ) as mock_project_hint, + patch( + "apm_cli.install.phases.finalize._hint_global_root_context", + ) as mock_global_hint, + ): + run(ctx) + + mock_project_hint.assert_called_once_with(ctx) + mock_global_hint.assert_not_called() diff --git a/tests/unit/install/test_finalize_phase.py b/tests/unit/install/test_finalize_phase.py index 7d54f22df..1e8b9fe8c 100644 --- a/tests/unit/install/test_finalize_phase.py +++ b/tests/unit/install/test_finalize_phase.py @@ -22,6 +22,7 @@ class _FakeCtx: project_root: Path = Path("/fake/root") apm_dir: Path = Path("/fake/root/.apm") + apm_modules_dir: Any = None installed_count: int = 0 unpinned_count: int = 0 installed_packages: list[Any] = field(default_factory=list) @@ -35,6 +36,8 @@ class _FakeCtx: logger: Any = None package_types: dict[str, str] = field(default_factory=dict) scope: Any = field(default_factory=lambda: InstallScope.PROJECT) + dry_run: bool = False + targets: list[Any] = field(default_factory=list) # ---------------------------------------------------------------------------