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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- `apm install` now honors manifest `targets:` without falling back to the legacy Copilot target when singular `target:` is absent. (#1560)

## [0.16.0] - 2026-05-28

### Added
Expand Down
88 changes: 68 additions & 20 deletions src/apm_cli/install/phases/targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,54 @@
from apm_cli.integration.targets import TargetProfile


def _package_field(apm_package: Any, name: str) -> Any:
"""Return a real APMPackage field without treating MagicMock attrs as set."""
if apm_package is None:
return None
try:
attrs = vars(apm_package)
except TypeError:
attrs = {}
if name in attrs:
return attrs[name]
value = getattr(apm_package, name, None)
if type(value).__module__ == "unittest.mock":
return None
return value


def _package_target_value(apm_package: Any) -> str | list[str] | None:
"""Read singular target or plural targets from the parsed package model."""
from apm_cli.core.apm_yml import parse_targets_field

target = _package_field(apm_package, "target")
targets = _package_field(apm_package, "targets")
if target is not None and targets is not None:
parse_targets_field({"target": target, "targets": targets})
if targets is not None:
parsed = parse_targets_field({"targets": targets})
return parsed if parsed else None
return target


def _raise_target_usage_error(ctx: Any, exc: Exception) -> None:
"""Render target field user errors consistently before exiting."""
if ctx.logger:
ctx.logger.error(str(exc), symbol="")
raise SystemExit(2) from exc


def _as_yaml_targets(value: str | list[str] | None) -> list[str] | None:
"""Normalize a package target value to the v2 yaml_targets shape."""
if value is None:
return None
if isinstance(value, str):
parts = [t.strip() for t in value.split(",") if t.strip()]
else:
parts = [str(t).strip() for t in value if str(t).strip()]
return parts or None


def _read_yaml_targets(ctx) -> list[str] | None:
"""Read targets/target from raw apm.yml using v2 parser.

Expand All @@ -29,10 +77,10 @@ def _read_yaml_targets(ctx) -> list[str] | None:
return None
apm_yml_path = getattr(ctx.apm_package, "package_path", None)
if apm_yml_path is None:
return None
return _as_yaml_targets(_package_target_value(ctx.apm_package))
manifest = apm_yml_path / "apm.yml"
if not manifest.exists():
return None
return _as_yaml_targets(_package_target_value(ctx.apm_package))
try:
from apm_cli.utils.yaml_io import load_yaml

Expand Down Expand Up @@ -92,6 +140,8 @@ def run(ctx: InstallContext) -> None:
On return ``ctx.targets`` and ``ctx.integrators`` are populated.
"""

import click as _click

from apm_cli.core.scope import InstallScope
from apm_cli.core.target_detection import (
detect_target,
Expand All @@ -113,8 +163,11 @@ def run(ctx: InstallContext) -> None:
resolve_targets as _resolve_targets_legacy,
)

# Get config target from apm.yml if available
config_target = ctx.apm_package.target
# Get config target from apm.yml if available.
try:
config_target = _package_target_value(ctx.apm_package)
except _click.UsageError as exc:
_raise_target_usage_error(ctx, exc)

# Resolve effective explicit target: CLI --target wins, then apm.yml
_explicit = ctx.target_override or config_target or None
Comment on lines +166 to 173
Expand Down Expand Up @@ -303,18 +356,12 @@ def run(ctx: InstallContext) -> None:
# Read targets from apm.yml (supports both target: and targets:)
_v2_yaml: list[str] | None = None
if _v2_flag is None and not ctx.target_override:
import click as _click

try:
_v2_yaml = _read_yaml_targets(ctx)
except _click.UsageError as exc:
# ConflictingTargetsError (both target: and targets: in
# apm.yml) is a user error -- surface with exit code 2.
# The renderer already emits a leading "[x]"; pass an
# empty symbol so logger.error doesn't double-prefix.
if ctx.logger:
ctx.logger.error(str(exc), symbol="")
raise SystemExit(2) from exc
_raise_target_usage_error(ctx, exc)

# Skip v2 entirely when all override targets were non-canonical
# (e.g. copilot-cowork only). Those are fully handled by the
Expand Down Expand Up @@ -400,7 +447,9 @@ def run(ctx: InstallContext) -> None:
# Keep any legacy-only targets (e.g. copilot-cowork) that v2
# doesn't handle.
_v2_names = {t.name for t in _v2_targets}
_legacy_only = [t for t in _targets if t.name not in _v2_names]
_legacy_only = [
t for t in _targets if t.name not in _v2_names and t.name not in _CANONICAL
]
_targets = _v2_targets + _legacy_only

else:
Expand Down Expand Up @@ -479,6 +528,8 @@ def run_targets_phase(ctx) -> None:
"""
from pathlib import Path

import click as _click

from apm_cli.core.target_detection import resolve_targets
from apm_cli.integration.targets import KNOWN_TARGETS

Expand All @@ -494,14 +545,11 @@ def run_targets_phase(ctx) -> None:
else:
flag = ctx.target_override

# Get yaml_targets from apm_package
yaml_targets: list[str] | None = None
if ctx.apm_package and ctx.apm_package.target:
raw = ctx.apm_package.target
if isinstance(raw, str):
yaml_targets = [t.strip() for t in raw.split(",") if t.strip()]
elif isinstance(raw, list):
yaml_targets = raw
# Get yaml_targets from apm_package.
try:
yaml_targets = _as_yaml_targets(_package_target_value(ctx.apm_package))
except _click.UsageError as exc:
_raise_target_usage_error(ctx, exc)

# Resolve targets
resolved = resolve_targets(project_root, flag=flag, yaml_targets=yaml_targets)
Expand Down
47 changes: 47 additions & 0 deletions tests/unit/install/phases/test_targets_phase.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,56 @@ def _make_ctx(
ctx.logger = MagicMock()
ctx.targets = []
ctx.integrators = {}
ctx.legacy_skill_paths = False
return ctx


def test_plural_targets_without_singular_does_not_keep_legacy_copilot_fallback(
tmp_path: Path,
) -> None:
"""targets: [claude] must not inherit legacy greenfield copilot fallback."""
from apm_cli.install.phases.targets import run
from apm_cli.models.apm_package import APMPackage

project = tmp_path / "project"
project.mkdir()
(project / "apm.yml").write_text(
"name: demo\nversion: 0.1.0\ntargets:\n - claude\n",
encoding="utf-8",
)
ctx = _make_ctx(tmp_path)
ctx.project_root = project
ctx.apm_package = APMPackage.from_apm_yml(project / "apm.yml")

run(ctx)

assert [target.name for target in ctx.targets] == ["claude"]
assert (project / ".claude").is_dir()
assert not (project / ".github").exists()


def test_run_conflicting_target_fields_exits_with_usage_code(tmp_path: Path) -> None:
"""target + targets conflicts must stay on the targets-phase error path."""
from apm_cli.install.phases.targets import run
from apm_cli.models.apm_package import APMPackage

project = tmp_path / "project"
project.mkdir()
(project / "apm.yml").write_text(
"name: demo\nversion: 0.1.0\ntarget: claude\ntargets:\n - copilot\n",
encoding="utf-8",
)
ctx = _make_ctx(tmp_path)
ctx.project_root = project
ctx.apm_package = APMPackage.from_apm_yml(project / "apm.yml")

with pytest.raises(SystemExit) as exc_info:
run(ctx)

assert exc_info.value.code == 2
ctx.logger.error.assert_called_once()


# ---------------------------------------------------------------------------
# TestProjectScopeGateForCowork
# ---------------------------------------------------------------------------
Expand Down
34 changes: 34 additions & 0 deletions tests/unit/install/phases/test_targets_phase_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def _make_ctx(
*,
target_override: str | None = None,
yaml_target: str | None = None,
yaml_targets: list[str] | None = None,
) -> MagicMock:
ctx = MagicMock()
ctx.project_root = project_root
Expand All @@ -40,6 +41,8 @@ def _make_ctx(
ctx.target_override = target_override
ctx.apm_package = MagicMock()
ctx.apm_package.target = yaml_target
if yaml_targets is not None:
ctx.apm_package.targets = yaml_targets
ctx.logger = MagicMock()
ctx.targets = []
ctx.integrators = {}
Expand Down Expand Up @@ -94,6 +97,37 @@ def test_explicit_creates_missing_dir(tmp_path):
assert (project / ".claude").is_dir(), "Explicit --target claude must materialize .claude/"


def test_plural_yaml_targets_attribute_creates_only_declared_dir(tmp_path):
"""targets: from the parsed APMPackage model drives v2 target selection."""
from apm_cli.install.phases.targets import run_targets_phase

project = tmp_path / "project"
project.mkdir()

ctx = _make_ctx(project, yaml_targets=["claude"])
run_targets_phase(ctx)

assert [target.name for target in ctx.targets] == ["claude"]
assert (project / ".claude").is_dir()
assert not (project / ".github").exists()


def test_run_targets_phase_conflicting_target_fields_exits_with_usage_code(tmp_path: Path) -> None:
"""run_targets_phase preserves usage-error exit code for target conflicts."""
from apm_cli.install.phases.targets import run_targets_phase

project = tmp_path / "project"
project.mkdir()

ctx = _make_ctx(project, yaml_target="claude", yaml_targets=["copilot"])

with pytest.raises(SystemExit) as exc_info:
run_targets_phase(ctx)

assert exc_info.value.code == 2
ctx.logger.error.assert_called_once()


@pytest.mark.parametrize(
("marker_path", "expected_dir"),
[
Expand Down
Loading