fix(install): accept YAML list form under singular target: key (#1188)

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Stabilized `test_install_over_defer_threshold_starts_live_once` on slow CI runners by joining the deferred-start timer thread instead of relying on a 100ms grace window. (#1191)
 - `triage-panel` scheduled sweep now paginates the candidate query oldest-first via the GitHub MCP `list_issues` tool instead of a single 200-issue page, so daily runs actually drain the untriaged backlog rather than processing one issue per cron tick. (#1193)
 - `triage-panel` scheduled sweep switches the candidate query from `list_issues`+prose-driven pagination to `search_issues` with `-label:status/triaged sort:created-asc`, so untriaged candidates are filtered server-side; the previous approach silently noop'd because the MCP gateway DIFC filter dropped non-collaborator issues mid-page and the agent inferred a false `hasNextPage:false`.
+- `apm install` now accepts the YAML list form under `target:` (e.g. `target: [copilot, claude]` or block-list), which previously crashed with `Unknown target '['copilot''` because the install-pipeline parser stringified the list before splitting; the unknown-target error renderer also strips bracket/quote noise from the headline value and suggests `copilot` instead of the alphabetically-first `agent-skills`. (#1197)
 
 ## [0.12.4] - 2026-05-07
 

src/apm_cli/core/apm_yml.py

@@ -4,6 +4,7 @@
   - 'targets: [a, b]'  -> ['a', 'b']   (canonical, plural)
   - 'target: a'        -> ['a']         (singular sugar)
   - 'target: "a,b"'    -> ['a', 'b']   (CSV sugar)
+  - 'target: [a, b]'   -> ['a', 'b']   (list sugar under singular key, #1188)
   - both present       -> raise ConflictingTargetsError
   - neither present    -> []            (empty = auto-detect upstream)
 
@@ -85,6 +86,15 @@ def parse_targets_field(yaml_data: dict) -> list[str]:
         raw = yaml_data["target"]
         if raw is None:
             return []
+        if isinstance(raw, list):
+            # YAML list sugar: 'target: [claude, copilot]' or block list.
+            # Empty list under singular key falls through to auto-detect
+            # (consistent with 'target:' with no value).
+            tokens = [str(t).strip() for t in raw if str(t).strip()]
+            if not tokens:
+                return []
+            _validate_canonical(tokens)
+            return tokens
         raw_str = str(raw).strip()
         if not raw_str:
             return []

src/apm_cli/core/errors.py

@@ -117,21 +117,26 @@ def render_ambiguous_error(project_root: Path | None, detected: list[str]) -> st
 def render_unknown_target_error(value: str, valid: list[str]) -> str:
     """Render the 3-section error for unknown target token."""
     valid_csv = ", ".join(sorted(valid))
+    # Strip bracket/quote noise that can leak in from misparsed tokens
+    # (e.g. "['copilot'"). Defense-in-depth: callers should pass clean
+    # values, but this keeps the headline readable if they don't.
+    display_value = value.strip("[]'\" ")
+    suggestion = "copilot" if "copilot" in valid else (valid[0] if valid else "claude")
     return (
-        f"[x] Unknown target '{value}'\n"
+        f"[x] Unknown target '{display_value}'\n"
         "\n"
         f"Valid targets: {valid_csv}\n"
         "\n"
         "Fix with one of:\n"
         "\n"
         "  apm targets                            # see all supported harnesses\n"
-        f"  apm install <pkg> --target {valid[0] if valid else 'claude'}\n"
+        f"  apm install <pkg> --target {suggestion}\n"
         "  apm install <pkg> --dry-run\n"
         "\n"
         "Or declare in apm.yml:\n"
         "\n"
         "  targets:\n"
-        f"    - {valid[0] if valid else 'claude'}"
+        f"    - {suggestion}"
     )
 
 

src/apm_cli/install/phases/targets.py

@@ -208,8 +208,10 @@ def run(ctx: InstallContext) -> None:
             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
+                # symbol=None so logger.error doesn't double-prefix.
                 if ctx.logger:
-                    ctx.logger.error(str(exc))
+                    ctx.logger.error(str(exc), symbol=None)
                 raise SystemExit(2) from exc
 
         # Skip v2 entirely when all override targets were non-canonical
@@ -246,9 +248,11 @@ def run(ctx: InstallContext) -> None:
                 # is a false positive because v2 does not handle
                 # non-canonical targets.  That case is already
                 # guarded by ``_skip_v2`` above, so it never reaches
-                # this except block.
+                # this except block.  The renderer already emits a
+                # leading "[x]"; pass symbol=None so logger.error
+                # doesn't double-prefix.
                 if ctx.logger:
-                    ctx.logger.error(str(exc))
+                    ctx.logger.error(str(exc), symbol=None)
                 raise SystemExit(2) from exc
 
             # Emit provenance BEFORE any mutation. Route via _rich_info so

tests/unit/core/test_error_renderer.py

@@ -71,6 +71,26 @@ def test_unknown_target_error_lists_valid():
     assert "claude" in text
 
 
+def test_unknown_target_error_suggests_copilot_not_first_alphabetical():
+    """Suggestion must be a sensible default (#1188), not sorted-first."""
+    valid = ["agent-skills", "claude", "copilot", "cursor"]
+    text = render_unknown_target_error("foo", valid)
+    # 'agent-skills' is alphabetically first but is the wrong default to
+    # surface to a user who typed 'foo'. Prefer 'copilot'.
+    assert "--target copilot" in text
+    assert "    - copilot" in text
+    assert "--target agent-skills" not in text
+
+
+def test_unknown_target_error_sanitizes_garbled_value():
+    """Headline must not show Python list-repr noise (#1188)."""
+    # Simulates the pre-fix garbled token "['copilot'" leaking through.
+    text = render_unknown_target_error("['copilot'", ["claude", "copilot"])
+    headline = text.splitlines()[0]
+    # Bracket and quote noise is stripped from the headline value.
+    assert headline == "[x] Unknown target 'copilot'"
+
+
 def test_conflicting_schema_error_has_three_parts():
     text = render_conflicting_schema_error()
     _assert_three_sections(text, ["cannot use both", "conflicting"])

tests/unit/core/test_target_resolution_v2.py

@@ -200,6 +200,68 @@ def test_unknown_target_rejected():
     assert "claude" in msg or "copilot" in msg
 
 
+# ---------------------------------------------------------------------------
+# YAML list under 'target:' singular key (#1188)
+# ---------------------------------------------------------------------------
+
+
+def test_target_singular_with_yaml_list_two_items():
+    """Regression: 'target: [copilot, claude]' YAML flow-list form (#1188)."""
+    out = parse_targets_field({"target": ["copilot", "claude"]})
+    assert sorted(out) == ["claude", "copilot"]
+
+
+def test_target_singular_with_yaml_list_single_item():
+    """Single-item list under 'target:' must equal scalar form."""
+    out = parse_targets_field({"target": ["copilot"]})
+    assert out == ["copilot"]
+
+
+def test_target_singular_with_yaml_list_whitespace_tolerated():
+    """List elements with surrounding whitespace are stripped."""
+    out = parse_targets_field({"target": ["  copilot  ", "claude\t"]})
+    assert sorted(out) == ["claude", "copilot"]
+
+
+def test_target_singular_with_empty_list_falls_through_to_autodetect():
+    """'target: []' under SINGULAR key returns [] (auto-detect upstream),
+    matching 'target:' with no value. Only PLURAL 'targets: []' raises."""
+    out = parse_targets_field({"target": []})
+    assert out == []
+
+
+def test_target_singular_with_yaml_list_unknown_token_rejected():
+    """Garbled tokens from list parsing must surface a clean error."""
+    with pytest.raises(UnknownTargetError) as exc_info:
+        parse_targets_field({"target": ["nonsense"]})
+    msg = str(exc_info.value)
+    headline = msg.splitlines()[0]
+    # Headline must contain the bare token, not a Python list repr.
+    assert "'nonsense'" in headline
+    # No list-repr leakage: the leading "[x]" symbol is fine, but no
+    # "['nonsense'" or similar should appear as the value.
+    assert "['nonsense'" not in headline
+    assert "[\"nonsense\"" not in headline
+
+
+def test_target_singular_with_yaml_list_non_string_coerced():
+    """Non-string list elements coerce via str() and are validated."""
+    with pytest.raises(UnknownTargetError):
+        parse_targets_field({"target": [42]})
+
+
+def test_target_singular_with_all_token_in_list_rejected():
+    """'all' is a CLI flag-only meta-target; must not validate inside YAML."""
+    with pytest.raises(UnknownTargetError):
+        parse_targets_field({"target": ["all", "claude"]})
+
+
+def test_target_singular_with_yaml_list_preserves_duplicates():
+    """Duplicates are preserved (parser does not dedup)."""
+    out = parse_targets_field({"target": ["copilot", "copilot"]})
+    assert out == ["copilot", "copilot"]
+
+
 # ---------------------------------------------------------------------------
 # --target all expansion
 # ---------------------------------------------------------------------------

tests/unit/install/phases/test_read_yaml_targets_list_form.py

@@ -0,0 +1,121 @@
+"""Regression tests for #1188: install pipeline must accept YAML
+list form under the singular 'target:' key.
+
+Before the fix, the install path (``_read_yaml_targets`` ->
+``parse_targets_field``) called ``str(raw)`` on a Python list, producing
+the literal repr ``"['copilot', 'claude']"`` and then comma-splitting
+it into garbled tokens (``"['copilot'"``, ``"'claude']"``). The dry-run
+path used a different parser and silently passed, masking the bug.
+
+These tests exercise the install-pipeline read path directly so that
+any future divergence between the dry-run parser
+(``target_detection.parse_target_field``) and the install parser
+(``apm_yml.parse_targets_field``) is caught at unit-test level.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from unittest.mock import MagicMock
+
+import pytest
+
+
+def _ctx_with_apm_yml(tmp_path: Path, body: str) -> MagicMock:
+    project = tmp_path / "proj"
+    project.mkdir()
+    (project / "apm.yml").write_text(body, encoding="utf-8")
+    ctx = MagicMock()
+    ctx.apm_package = MagicMock()
+    ctx.apm_package.package_path = project
+    return ctx
+
+
+def test_read_yaml_targets_accepts_flow_list_under_singular_key(tmp_path):
+    """target: [copilot, claude] (flow style) must parse to a list."""
+    from apm_cli.install.phases.targets import _read_yaml_targets
+
+    ctx = _ctx_with_apm_yml(
+        tmp_path,
+        "name: demo\nversion: 0.1.0\ntarget: [copilot, claude]\n",
+    )
+    result = _read_yaml_targets(ctx)
+    assert result is not None
+    assert sorted(result) == ["claude", "copilot"]
+
+
+def test_read_yaml_targets_accepts_block_list_under_singular_key(tmp_path):
+    """target:\\n  - copilot\\n  - claude (block style) must parse to a list."""
+    from apm_cli.install.phases.targets import _read_yaml_targets
+
+    ctx = _ctx_with_apm_yml(
+        tmp_path,
+        "name: demo\nversion: 0.1.0\ntarget:\n  - copilot\n  - claude\n",
+    )
+    result = _read_yaml_targets(ctx)
+    assert result is not None
+    assert sorted(result) == ["claude", "copilot"]
+
+
+def test_read_yaml_targets_csv_form_still_works(tmp_path):
+    """Regression guard: CSV form 'target: copilot,claude' continues to work."""
+    from apm_cli.install.phases.targets import _read_yaml_targets
+
+    ctx = _ctx_with_apm_yml(
+        tmp_path,
+        'name: demo\nversion: 0.1.0\ntarget: "copilot,claude"\n',
+    )
+    result = _read_yaml_targets(ctx)
+    assert result is not None
+    assert sorted(result) == ["claude", "copilot"]
+
+
+def test_read_yaml_targets_scalar_form_still_works(tmp_path):
+    """Regression guard: scalar 'target: copilot' continues to work."""
+    from apm_cli.install.phases.targets import _read_yaml_targets
+
+    ctx = _ctx_with_apm_yml(
+        tmp_path,
+        "name: demo\nversion: 0.1.0\ntarget: copilot\n",
+    )
+    result = _read_yaml_targets(ctx)
+    assert result == ["copilot"]
+
+
+def test_read_yaml_targets_install_and_dry_run_parsers_agree(tmp_path):
+    """Both the install parser (parse_targets_field) and the dry-run
+    parser (parse_target_field) must ACCEPT the YAML list form without
+    crashing and return 2 targets. The two parsers canonicalize aliases
+    differently (e.g. copilot vs vscode), but that's tracked separately
+    as part of consolidating onto a single parser. The point of this
+    regression-trap is that both code paths agree on shape, not naming.
+    """
+    from apm_cli.core.apm_yml import parse_targets_field
+    from apm_cli.core.target_detection import parse_target_field
+
+    yaml_value = ["copilot", "claude"]
+    install_result = parse_targets_field({"target": yaml_value})
+    dry_run_result = parse_target_field(yaml_value)
+
+    assert len(install_result) == 2
+    assert len(dry_run_result) == 2
+    # claude is canonical in both parsers; this is the stable cross-check.
+    assert "claude" in install_result
+    assert "claude" in dry_run_result
+
+
+def test_read_yaml_targets_unknown_token_in_list_raises_clean_error(tmp_path):
+    """Unknown token inside a YAML list must surface a readable headline,
+    not a Python list-repr fragment like '['copilot''.
+    """
+    from apm_cli.core.errors import UnknownTargetError
+    from apm_cli.install.phases.targets import _read_yaml_targets
+
+    ctx = _ctx_with_apm_yml(
+        tmp_path,
+        "name: demo\nversion: 0.1.0\ntarget: [bogus]\n",
+    )
+    with pytest.raises(UnknownTargetError) as exc_info:
+        _read_yaml_targets(ctx)
+    headline = str(exc_info.value).splitlines()[0]
+    assert headline == "[x] Unknown target 'bogus'"