fix(targets): validate apm.yml target: at parse time, share normalization with CLI (closes #820)

CHANGELOG.md

@@ -10,11 +10,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **Manifest contract: invalid `target:` values now raise a parse error.** Previously, an unknown token (or a CSV string like `target: opencode,claude,copilot,agents` instead of the YAML list `target: [opencode, claude, copilot, agents]`) was silently ignored, leaving `apm install` and `apm compile` to exit 0 while deploying nothing. The shared parser used by `--target` now also validates `apm.yml`'s `target:`, so the same input resolves the same way at every entry point. **Migration:** three previously-silent inputs now fail loud -- (1) unknown tokens (`target: bogus` -> fix the typo), (2) empty values (`target: ""`, `target: []` -> remove the line if you meant auto-detect), (3) `all` mixed with other targets (`target: [all, claude]` -> use `all` alone). Omitting `target:` entirely still triggers auto-detection. (#820)
 - Rename `DownloadStrategyManager` to `DownloadDelegate` to better reflect Facade/Delegate pattern (#918)
 - Fix incorrect double-checked locking in marketplace registry `_load()` -- hold lock across full check+read+set (#918)
 
 ### Fixed
 
+- `apm install` and `apm compile` no longer exit 0 with success messages when `target:` in `apm.yml` is a CSV string -- the value now parses identically to the same input on `--target`, and zero-target resolution surfaces a warning instead of a silent no-op. (#820)
 - Remove redundant `seen` set from `_scan_patterns()` discovery walk (#918)
 
 ## [0.10.0] - 2026-04-27

docs/src/content/docs/reference/manifest-schema.md

@@ -113,7 +113,7 @@ policy:        <PolicyConfig>
 | **Default** | Auto-detect: `vscode` if `.github/` exists, `claude` if `.claude/` exists, `codex` if `.codex/` exists, `all` if multiple target folders exist, `minimal` if none |
 | **Allowed values** | `vscode` · `agents` · `copilot` · `claude` · `cursor` · `opencode` · `codex` · `all` |
 
-Controls which output targets are generated during compilation and installation. Accepts a single string or a list of strings. When unset, a conforming resolver SHOULD auto-detect based on folder presence. Unknown values MUST be silently ignored (auto-detection takes over).
+Controls which output targets are generated during compilation and installation. Accepts a single string or a list of strings. When unset, a conforming resolver SHOULD auto-detect based on folder presence. Unknown values MUST raise a parse error pointing at the offending token. Auto-detection applies only when `target:` is unset.
 
 ```yaml
 # Single target

packages/apm-guide/.apm/skills/apm-usage/package-authoring.md

@@ -58,6 +58,30 @@ my-package/
         resource2.md
 ```
 
+## Manifest fields: `target:` validation contract
+
+The `target:` field in `apm.yml` controls which output runtimes the package
+compiles and installs to. Both `apm.yml`'s `target:` and the `--target` CLI
+flag share the same validator, so identical input is rejected or accepted
+the same way at every entry point. Invalid values fail at parse time with a
+message naming the apm.yml path and the offending token -- they do **not**
+silently fall through to auto-detect.
+
+| Form | Behaviour |
+|------|-----------|
+| `target: copilot` | Single token; allowed values: `vscode`, `agents`, `copilot`, `claude`, `cursor`, `opencode`, `codex`, `all` |
+| `target: [claude, copilot]` | List form; only listed targets are compiled/installed |
+| `target: claude,copilot` | CSV-string form; parses identically to the list form (the shared validator splits on `,`). Before #820 was fixed, this silently produced zero deployment |
+| `target:` omitted entirely | Auto-detect from project folders (`.github/`, `.claude/`, `.codex/`) |
+| `target: bogus` (unknown token) | **Parse error** -- fix the typo |
+| `target: ""` or `target: []` (empty) | **Parse error** -- remove the line if you meant auto-detect |
+| `target: [all, claude]` (`all` mixed with other targets) | **Parse error** -- use `all` alone |
+
+Error messages always name the `apm.yml` path and the offending token, so the
+fix point is unambiguous. The list form (`target: [a, b]`) is the recommended
+shape; the CSV-string form is supported for parity with `--target a,b` on the
+CLI but reads less cleanly in YAML.
+
 ## The 7 primitive types
 
 ### 1. Instruction (`*.instructions.md`)

packages/apm-guide/.apm/skills/apm-usage/workflow.md

@@ -78,6 +78,16 @@ CLI equivalent: `--target claude,copilot` (comma-separated).
 | Multiple target folders | `all` |
 | Neither exists | `minimal` (AGENTS.md only) |
 
+Auto-detection only applies when `target:` is omitted entirely. Invalid `target:` values fail at parse time with a message naming the apm.yml path and the offending token. The same shared validator runs for both `apm.yml`'s `target:` and the `--target` CLI flag, so identical input produces identical results at every entry point.
+
+| Input | Result |
+|-------|--------|
+| `target: bogus` (unknown token) | parse error -- fix the typo |
+| `target: ""` or `target: []` (empty) | parse error -- remove the line if you meant auto-detect |
+| `target: [all, claude]` (`all` mixed with other targets) | parse error -- use `all` alone |
+| `target: opencode,claude,copilot,agents` (CSV string in YAML) | accepted; parses identically to the list form `target: [opencode, claude, copilot, agents]` (used to silently zero-deploy before #820 was fixed) |
+| `target:` line omitted | auto-detect from folders (table above) |
+
 ## What to commit
 
 | Path | Commit? | Why |

src/apm_cli/commands/compile/cli.py

@@ -379,16 +379,19 @@ def compile(
         # Auto-detect target if not explicitly provided
         from ...core.target_detection import detect_target, get_target_description
 
-        # Get config target from apm.yml if available
-        config_target = None
-        try:
-            from ...models.apm_package import APMPackage
+        # Get config target from apm.yml if available.  When the file is
+        # absent we proceed with auto-detection; when it is present but
+        # malformed we let the parse error surface so users see exactly
+        # what is wrong (e.g. ``target: opencode,bogus`` -> a ValueError
+        # naming the bad token), rather than silently falling through to
+        # auto-detect.  See #820.
+        from ...models.apm_package import APMPackage
 
-            apm_pkg = APMPackage.from_apm_yml(Path(APM_YML_FILENAME))
+        config_target = None
+        apm_yml_path = Path(APM_YML_FILENAME)
+        if apm_yml_path.exists():
+            apm_pkg = APMPackage.from_apm_yml(apm_yml_path)
             config_target = apm_pkg.target
-        except Exception:
-            # No apm.yml or parsing error - proceed with auto-detection
-            pass
 
         # Resolve list targets to compiler-understood string
         compile_target = _resolve_compile_target(target)
@@ -475,8 +478,39 @@ def compile(
                     # Success message for dry run already included in formatter output
                     pass
                 else:
-                    # Success message for actual compilation
-                    logger.success("Compilation completed successfully!", symbol="check")
+                    # Defense-in-depth (#820): don't claim "completed
+                    # successfully" when zero files were emitted.  With
+                    # parse_target_field as the upstream gatekeeper this is
+                    # unreachable in normal flow, but silent zero-effect
+                    # success is the worst-case package-manager DX.
+                    #
+                    # Pattern-based stat scan (instead of a hardcoded key
+                    # list) so new compile-time targets pick up the guard
+                    # automatically: any stat ending in ``_files_written``
+                    # or ``_files_generated`` contributes to the total.
+                    _files_written = sum(
+                        int(v or 0)
+                        for k, v in result.stats.items()
+                        if k.endswith(("_files_written", "_files_generated"))
+                    )
+                    if _files_written > 0:
+                        logger.success(
+                            "Compilation completed successfully!",
+                            symbol="check",
+                        )
+                    else:
+                        # Zero-output compile is the silent-success failure
+                        # mode #820 guards against.  Don't claim success;
+                        # surface what the user can act on.  The cause is
+                        # usually one of: target dirs not present (auto-
+                        # detect found nothing), explicit target rejected
+                        # by policy, or no primitives in the project.
+                        logger.warning(
+                            "Compilation completed but produced no output "
+                            "files. Check that target directories exist "
+                            "(e.g. .github/, .claude/) or set 'target:' "
+                            "in apm.yml / pass --target explicitly."
+                        )
 
             else:
                 # Traditional single-file compilation - keep existing logic

src/apm_cli/core/target_detection.py

@@ -261,12 +261,141 @@ def normalize_target_list(
 )
 
 
+def parse_target_field(
+    value: Union[str, List[str], None],
+    *,
+    source_path: Optional[Path] = None,
+) -> Union[str, List[str], None]:
+    """Parse, validate, and normalize a target value from any entry point.
+
+    Single source of truth for the ``target`` field, shared by the
+    ``--target`` CLI flag (via :class:`TargetParamType`) and ``apm.yml``'s
+    top-level ``target:`` (via :func:`APMPackage.from_apm_yml`).  The
+    output may differ from the input in case (lowercased), order
+    (preserved but deduplicated), and shape (single-element multi-token
+    inputs collapse to ``str``).  Aliases are resolved for multi-token
+    input only; see the *Returns* section below for the exact rules.
+
+    Accepted input shapes:
+
+    * ``None`` -> ``None`` (auto-detect at consumption time -- this is the
+      "field absent" path; an apm.yml without ``target:`` lands here).
+    * Single token (``"claude"``) -> the same lowercased token as ``str``.
+      Aliases are NOT resolved for solo input -- ``"copilot"`` returns
+      ``"copilot"`` (not the canonical ``"vscode"``) to match the
+      long-standing CLI contract; downstream consumers handle the alias
+      set explicitly.
+    * CSV string (``"claude,copilot"``) -> deduplicated ``List[str]`` with
+      aliases resolved to canonical names. Collapses to a bare ``str`` if
+      after dedup only one canonical token remains.
+    * List input (``["claude", "copilot"]``) goes through the same path as
+      the CSV form -- single-element lists collapse to ``str``.
+    * Literal ``"all"`` -> ``"all"`` (exclusive; cannot be combined).
+
+    Args:
+        value: The raw value -- ``str``, ``List[str]``, or ``None``.
+        source_path: Optional path to the apm.yml that produced ``value``.
+            When supplied, ValueError messages name the file so users can
+            jump to it directly.
+
+    Returns:
+        ``None`` for unset, a ``str`` for a single token (or ``"all"``),
+        or a deduplicated ``List[str]`` for multi-target input.
+
+    Raises:
+        ValueError: When the value is an empty / whitespace-only / commas-only
+            string, an empty list, a non-string non-list type, contains a
+            token that is not in :data:`VALID_TARGET_VALUES`, or mixes
+            ``"all"`` with other targets.  An empty *string* is treated as
+            user error (the "field absent" path is ``None``, supplied by
+            the YAML loader for a missing key).
+    """
+    if value is None:
+        return None
+
+    # ---- collect raw tokens ----
+    if isinstance(value, str):
+        # Empty / whitespace-only / comma-only strings are user error -- a
+        # missing field comes through as ``None`` from the YAML loader, so
+        # an empty *string* means the user typed something invalid.
+        raw_parts = [v.strip().lower() for v in value.split(",") if v.strip()]
+    elif isinstance(value, list):
+        raw_parts = []
+        for item in value:
+            if not isinstance(item, str):
+                raise ValueError(_target_error(
+                    f"each entry must be a string, got {type(item).__name__}",
+                    source_path,
+                ))
+            if item.strip():
+                raw_parts.append(item.strip().lower())
+    else:
+        raise ValueError(_target_error(
+            f"expected string or list of strings, got {type(value).__name__}",
+            source_path,
+        ))
+
+    if not raw_parts:
+        raise ValueError(_target_error("target value must not be empty", source_path))
+
+    # ---- validate every token ----
+    for p in raw_parts:
+        if p not in VALID_TARGET_VALUES:
+            raise ValueError(_target_error(
+                f"'{p}' is not a valid target. "
+                f"Choose from: {', '.join(sorted(VALID_TARGET_VALUES))}",
+                source_path,
+            ))
+
+    # ---- "all" is exclusive ----
+    if "all" in raw_parts:
+        if len(raw_parts) > 1:
+            raise ValueError(_target_error(
+                "'all' cannot be combined with other targets",
+                source_path,
+            ))
+        return "all"
+
+    # Single-token input is returned as-is (no alias resolution).  This
+    # preserves the long-standing CLI contract where ``--target copilot``
+    # yields ``"copilot"`` rather than the canonical ``"vscode"``; every
+    # downstream consumer (active_targets, agents_compiler,
+    # _CROSS_TARGET_MAPS, _TARGET_PREFIXES) already accepts both alias
+    # spellings, so resolving here would be a visible behaviour change
+    # with zero functional benefit and would break the CLI test suite
+    # (~10 ``test_single_*`` cases).  This is the one asymmetry #820's
+    # "shared normalization" intentionally leaves in place; collapsing it
+    # is an independent decision tracked separately from this fix.
+    if len(raw_parts) == 1:
+        return raw_parts[0]
+
+    # Multi-token: resolve aliases + dedupe, preserving input order.
+    seen: set[str] = set()
+    result: List[str] = []
+    for p in raw_parts:
+        canonical = TARGET_ALIASES.get(p, p)
+        if canonical not in seen:
+            seen.add(canonical)
+            result.append(canonical)
+
+    if len(result) == 1:
+        return result[0]
+    return result
+
+
+def _target_error(message: str, source_path: Optional[Path]) -> str:
+    """Format a target validation error, naming the source file when known."""
+    if source_path is not None:
+        return f"Invalid 'target' in {source_path}: {message}"
+    return f"Invalid target: {message}"
+
+
 class TargetParamType(click.ParamType):
     """Click parameter type accepting comma-separated target values.
 
-    Single values and ``"all"`` are returned as plain strings for backward
-    compatibility with existing command handlers.  Multiple comma-separated
-    targets are returned as a deduplicated ``list[str]`` of canonical names.
+    Delegates to :func:`parse_target_field`, which is the shared validator
+    used by ``apm.yml``'s ``target:`` field as well -- so ``--target X`` and
+    ``target: X`` always resolve identically and reject the same inputs.
 
     Examples::
 
@@ -284,50 +413,10 @@ def convert(
         param: Optional[click.Parameter],
         ctx: Optional[click.Context],
     ) -> Union[str, List[str], None]:
-        if value is None:
-            return None
-        # If already converted (e.g. from a default), pass through.
-        if isinstance(value, list):
-            return value
-
-        # Split on comma, normalize whitespace & case, drop empty parts.
-        parts = [v.strip().lower() for v in value.split(",") if v.strip()]
-        if not parts:
-            self.fail("target value must not be empty", param, ctx)
-
-        # Validate every token.
-        for p in parts:
-            if p not in VALID_TARGET_VALUES:
-                self.fail(
-                    f"'{p}' is not a valid target. "
-                    f"Choose from: {', '.join(sorted(VALID_TARGET_VALUES))}",
-                    param,
-                    ctx,
-                )
-
-        # "all" is exclusive -- reject combinations like "all,claude".
-        if "all" in parts:
-            if len(parts) > 1:
-                self.fail(
-                    "'all' cannot be combined with other targets",
-                    param,
-                    ctx,
-                )
-            return "all"
-
-        # Single target -> plain string (backward compat).
-        if len(parts) == 1:
-            return parts[0]
-
-        # Multi-target: resolve aliases and deduplicate.
-        seen: set[str] = set()
-        result: List[str] = []
-        for p in parts:
-            canonical = TARGET_ALIASES.get(p, p)
-            if canonical not in seen:
-                seen.add(canonical)
-                result.append(canonical)
-        # If aliases collapsed everything to one target, return a string.
-        if len(result) == 1:
-            return result[0]
-        return result
+        try:
+            return parse_target_field(value)
+        except ValueError as e:
+            # Click idiom: route validation errors through self.fail so the
+            # user sees a clean "Invalid value for '--target': ..." message
+            # rather than a Python traceback.
+            self.fail(str(e), param, ctx)

src/apm_cli/install/phases/targets.py

@@ -115,21 +115,30 @@ def run(ctx: "InstallContext") -> None:
                 )
             raise SystemExit(1)
 
-    # Log target detection results
-    if ctx.logger and _targets:
+    # Log target detection results.  The empty-targets branch is a defensive
+    # warning -- with parse_target_field as the upstream gatekeeper this
+    # state is unreachable in normal flow, but a silent zero-target install
+    # is the worst-case package-manager DX (see #820), so always emit.
+    if ctx.logger:
         _scope_label = "global" if _is_user else "project"
-        _target_names = ", ".join(
-            f"{t.name} (~/{t.root_dir}/)" if _is_user else t.name
-            for t in _targets
-        )
-        ctx.logger.verbose_detail(
-            f"Active {_scope_label} targets: {_target_names}"
-        )
-        if _is_user:
-            from apm_cli.deps.lockfile import get_lockfile_path
-
+        if _targets:
+            _target_names = ", ".join(
+                f"{t.name} (~/{t.root_dir}/)" if _is_user else t.name
+                for t in _targets
+            )
             ctx.logger.verbose_detail(
-                f"Lockfile: {get_lockfile_path(ctx.apm_dir)}"
+                f"Active {_scope_label} targets: {_target_names}"
+            )
+            if _is_user:
+                from apm_cli.deps.lockfile import get_lockfile_path
+
+                ctx.logger.verbose_detail(
+                    f"Lockfile: {get_lockfile_path(ctx.apm_dir)}"
+                )
+        else:
+            ctx.logger.warning(
+                f"No {_scope_label} targets resolved -- nothing will be "
+                f"deployed. Check 'target:' in apm.yml or use --target."
             )
 
     for _t in _targets: