Skip to content
Merged
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
27 changes: 14 additions & 13 deletions src/apm_cli/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -971,19 +971,20 @@ def _log_integration(msg):
deployed.append(tp.relative_to(project_root).as_posix())

# --- commands (.claude) ---
command_result = command_integrator.integrate_package_commands(
package_info, project_root,
force=force, managed_files=managed_files,
diagnostics=diagnostics,
)
if command_result.files_integrated > 0:
result["commands"] += command_result.files_integrated
_log_integration(f" └─ {command_result.files_integrated} commands integrated -> .claude/commands/")
if command_result.files_updated > 0:
_log_integration(f" └─ {command_result.files_updated} commands updated")
result["links_resolved"] += command_result.links_resolved
for tp in command_result.target_paths:
deployed.append(tp.relative_to(project_root).as_posix())
if integrate_claude:
command_result = command_integrator.integrate_package_commands(
package_info, project_root,
force=force, managed_files=managed_files,
diagnostics=diagnostics,
Comment on lines +974 to +978

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

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

Please add/adjust a unit test to cover this regression: when the detected/configured target yields integrate_claude=False (e.g., target=vscode/copilot), apm install should not call integrate_package_commands and should not create/deploy anything under .claude/commands/. This will prevent future accidental reintroduction of unconditional Claude command integration.

Copilot uses AI. Check for mistakes.

@neop26 neop26 Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — added TestIntegratePackagePrimitivesTargetGating in test_command_integrator.py with two tests: one verifying integrate_package_commands is not called when integrate_claude=False (target=copilot/vscode), and one verifying it is called when integrate_claude=True.

)
if command_result.files_integrated > 0:
result["commands"] += command_result.files_integrated
_log_integration(f" └─ {command_result.files_integrated} commands integrated -> .claude/commands/")
if command_result.files_updated > 0:
_log_integration(f" └─ {command_result.files_updated} commands updated")
Comment on lines +982 to +984

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

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

These log strings include Unicode box-drawing characters ("└" and "─"), which violates the repo's ASCII-only encoding rule for source/CLI output (see .github/instructions/encoding.instructions.md). Please switch this message prefix to plain ASCII (for example, a simple "-" or the bracket status symbols) so Windows cp1252 terminals do not crash with UnicodeEncodeError.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These log strings match the existing pattern used throughout _integrate_package_primitives() — all ~15 existing _log_integration calls on lines 849, 864, 879, etc. use the same └─ character. Changing only the new lines would create an inconsistency. If there's a repo-wide ASCII encoding requirement, I'm happy to raise a separate PR to update all occurrences consistently.

result["links_resolved"] += command_result.links_resolved
for tp in command_result.target_paths:
deployed.append(tp.relative_to(project_root).as_posix())

# --- OpenCode commands (.opencode) ---
opencode_command_result = command_integrator.integrate_package_commands_opencode(
Expand Down
119 changes: 119 additions & 0 deletions tests/unit/integration/test_command_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,3 +365,122 @@ def test_sync_handles_missing_dir(self, temp_project_no_opencode):
integrator = CommandIntegrator()
result = integrator.sync_integration_opencode(None, temp_project_no_opencode)
assert result["files_removed"] == 0


class TestIntegratePackagePrimitivesTargetGating:
"""Tests that _integrate_package_primitives respects the integrate_claude flag.

Regression test for: CommandIntegrator was called unconditionally, causing
.claude/commands/ to be created even when target=copilot (integrate_claude=False).
"""

def _make_mock_integrators(self):
"""Return a dict of MagicMock integrators for _integrate_package_primitives."""
from unittest.mock import MagicMock

def _empty_result(*args, **kwargs):
r = MagicMock()
r.files_integrated = 0
r.files_updated = 0
r.links_resolved = 0
r.target_paths = []
r.skill_created = False
r.sub_skills_promoted = 0
r.hooks_integrated = 0
return r

integrators = {}
for name in (
"prompt_integrator",
"agent_integrator",
"skill_integrator",
"instruction_integrator",
"command_integrator",
"hook_integrator",
):
m = MagicMock()
for method in (
"integrate_package_prompts",
"integrate_package_agents",
"integrate_package_agents_claude",
"integrate_package_agents_cursor",
"integrate_package_agents_opencode",
"integrate_package_skill",
"integrate_package_instructions",
"integrate_package_instructions_cursor",
"integrate_package_commands",
"integrate_package_commands_opencode",
"integrate_package_hooks",
"integrate_package_hooks_claude",
"integrate_package_hooks_cursor",
):
getattr(m, method).side_effect = _empty_result
integrators[name] = m
return integrators

def test_integrate_claude_false_does_not_call_integrate_package_commands(self):
"""When integrate_claude=False, integrate_package_commands must not be called.

This is the regression test for the bug where .claude/commands/ was created
even when target=copilot (vscode) set integrate_claude=False.
"""
import tempfile, shutil
from apm_cli.commands.install import _integrate_package_primitives
from apm_cli.utils.diagnostics import DiagnosticCollector

temp_dir = tempfile.mkdtemp()
try:
project_root = Path(temp_dir)
(project_root / ".github").mkdir()

package_info = MagicMock()
integrators = self._make_mock_integrators()
diagnostics = DiagnosticCollector(verbose=False)

_integrate_package_primitives(
package_info,
project_root,
integrate_vscode=True,
integrate_claude=False,
integrate_opencode=False,
managed_files=set(),
force=False,
diagnostics=diagnostics,
**integrators,
)

integrators["command_integrator"].integrate_package_commands.assert_not_called()
assert not (project_root / ".claude" / "commands").exists()
finally:
shutil.rmtree(temp_dir, ignore_errors=True)

def test_integrate_claude_true_calls_integrate_package_commands(self):
"""When integrate_claude=True, integrate_package_commands must be called."""
import tempfile, shutil
from apm_cli.commands.install import _integrate_package_primitives
from apm_cli.utils.diagnostics import DiagnosticCollector

temp_dir = tempfile.mkdtemp()
try:
project_root = Path(temp_dir)
(project_root / ".claude").mkdir()

package_info = MagicMock()
integrators = self._make_mock_integrators()
diagnostics = DiagnosticCollector(verbose=False)

_integrate_package_primitives(
package_info,
project_root,
integrate_vscode=False,
integrate_claude=True,
integrate_opencode=False,
managed_files=set(),
force=False,
diagnostics=diagnostics,
**integrators,
)

integrators["command_integrator"].integrate_package_commands.assert_called_once()
finally:
shutil.rmtree(temp_dir, ignore_errors=True)
Loading