fix(install): honor targets: (plural) without singular target: present (closes #1503) (#1560)

* fix(install): honor plural targets without copilot fallback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(install): preserve target conflict usage errors

Ensure plural-target parsing errors raised from the package model stay on the targets-phase UsageError path with logger formatting and exit code 2, addressing Copilot inline review and panel follow-ups for PR #1560. Add regression traps for run() and run_targets_phase(), plus the release-note entry for issue #1503.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: danielmeppiel

CHANGELOG.md

@@ -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

src/apm_cli/install/phases/targets.py

@@ -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.
 
@@ -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
 
@@ -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,
@@ -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
@@ -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
@@ -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:
@@ -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
 
@@ -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)

tests/unit/install/phases/test_targets_phase.py

@@ -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
 # ---------------------------------------------------------------------------

tests/unit/install/phases/test_targets_phase_v2.py

@@ -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
@@ -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 = {}
@@ -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"),
     [