Skip to content

Commit 7477066

Browse files
AddonoCopilotdanielmeppiel
authored
fix(install): keep copilot in multi-target --target lists (#1749)
* fix(install): keep copilot in multi-target --target lists parse_target_field resolves multi-token input through TARGET_ALIASES (copilot -> vscode), but _resolve_targets_by_scope filtered flag tokens against CANONICAL_TARGETS, which has copilot and not vscode -- so the token was silently dropped and only the other targets installed. Normalize runtime aliases (vscode/agents/intellij -> copilot) through the shared RUNTIME_TO_CANONICAL_TARGET table before the canonical filter, mirroring the mcp_integrator precedent. Closes #1746 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: clarify alias-vs-canonical contract in #1746 regression trap Copilot review: docstring said the parse result keeps copilot while the assertion checks for the vscode alias. Split the two layers explicitly: parse returns the runtime alias, the targets phase normalizes it back. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: fold review followups for target aliases Document the first-seen ordering contract on the runtime target alias helper and credit the community PR in the changelog. These fold the apm-review-panel followups without changing install behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com>
1 parent b8fc5e3 commit 7477066

3 files changed

Lines changed: 122 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3232

3333
### Fixed
3434

35+
- `apm install --target` with a comma-separated list containing `copilot` (or the `vscode`/`agents` aliases) no longer silently drops the Copilot target. (by @Addono, closes #1746, #1749)
3536
- `apm install` now resolves relative `path:` deps declared by remote monorepo packages when they stay inside the same remote repo, while still rejecting absolute, escaping, or cross-repo paths; closes #1571. (#1732)
3637
- Optional-auth MCP registry servers now install without token prompts when values are unset; `apm install` also omits empty runtime config entries and preserves user-edited optional values on reinstall; refs #20. (#1734)
3738
- Dependencies with the same path on different git hosts no longer collide in `apm.lock.yaml`; `apm install` keeps GitHub/GitLab PATs off generic-host file downloads, routes bespoke GitLab hosts through `type: gitlab`, and surfaces non-404 download failures with host and endpoint context. Reading private files from a generic non-default host now requires a whole-repo git dependency or explicit backend signal; see the dependency and lockfile docs (closes #773). (#1735)

src/apm_cli/install/phases/targets.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,24 @@ def _as_yaml_targets(value: str | list[str] | None) -> list[str] | None:
6767
return parts or None
6868

6969

70+
def _normalize_runtime_target_aliases(tokens: Iterable[str]) -> list[str]:
71+
"""Map runtime aliases to canonical target names in first-seen order."""
72+
from apm_cli.integration.targets import RUNTIME_TO_CANONICAL_TARGET
73+
74+
normalized: list[str] = []
75+
seen: set[str] = set()
76+
for token in tokens:
77+
raw = str(token).strip()
78+
if not raw:
79+
continue
80+
canonical = RUNTIME_TO_CANONICAL_TARGET.get(raw, raw)
81+
if canonical in seen:
82+
continue
83+
seen.add(canonical)
84+
normalized.append(canonical)
85+
return normalized
86+
87+
7088
def _read_yaml_targets(ctx) -> list[str] | None:
7189
"""Read targets/target from raw apm.yml using v2 parser.
7290
@@ -357,6 +375,8 @@ def _fmt_target(t: Any) -> str:
357375
parts = [t.strip() for t in raw_override.split(",") if t.strip()]
358376
else:
359377
parts = list(raw_override)
378+
# Multi-token CLI parsing returns runtime aliases; convert them before filtering.
379+
parts = _normalize_runtime_target_aliases(parts)
360380
parts = [p for p in parts if p in _CANONICAL]
361381
if len(parts) == 1:
362382
_v2_flag = parts[0]
@@ -551,9 +571,10 @@ def run_targets_phase(ctx) -> None:
551571
if isinstance(ctx.target_override, str):
552572
# Handle CSV form
553573
parts = [t.strip() for t in ctx.target_override.split(",") if t.strip()]
554-
flag = parts if len(parts) > 1 else parts[0] if parts else None
555574
else:
556-
flag = ctx.target_override
575+
parts = list(ctx.target_override)
576+
parts = _normalize_runtime_target_aliases(parts)
577+
flag = parts if len(parts) > 1 else parts[0] if parts else None
557578

558579
# Get yaml_targets from apm_package.
559580
try:

tests/unit/install/phases/test_targets_phase_v2.py

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
def _make_ctx(
3131
project_root: Path,
3232
*,
33-
target_override: str | None = None,
33+
target_override: str | list[str] | None = None,
3434
yaml_target: str | None = None,
3535
yaml_targets: list[str] | None = None,
3636
) -> MagicMock:
@@ -46,9 +46,15 @@ def _make_ctx(
4646
ctx.logger = MagicMock()
4747
ctx.targets = []
4848
ctx.integrators = {}
49+
ctx.legacy_skill_paths = False
4950
return ctx
5051

5152

53+
def _target_names(ctx: MagicMock) -> list[str]:
54+
"""Return the resolved target profile names from a phase context."""
55+
return [target.name for target in ctx.targets]
56+
57+
5258
def _target_root_dirs(ctx, project_root: Path) -> list[Path]:
5359
"""Collect on-disk deploy directories for every TargetProfile in ctx.targets.
5460
@@ -68,6 +74,97 @@ def _target_root_dirs(ctx, project_root: Path) -> list[Path]:
6874
return out
6975

7076

77+
def test_runtime_alias_list_preserves_copilot_profile_and_dirs(tmp_path: Path) -> None:
78+
"""Multi-target runtime aliases resolve through the copilot profile."""
79+
from apm_cli.install.phases.targets import run
80+
81+
project = tmp_path / "project"
82+
project.mkdir()
83+
84+
ctx = _make_ctx(project, target_override=["claude", "vscode"])
85+
run(ctx)
86+
87+
assert _target_names(ctx) == ["claude", "copilot"]
88+
assert (project / ".claude").is_dir()
89+
assert (project / ".github").is_dir()
90+
91+
92+
@pytest.mark.parametrize("target_override", [["vscode"], "vscode"])
93+
def test_run_targets_phase_normalizes_vscode_alias(
94+
tmp_path: Path,
95+
target_override: str | list[str],
96+
) -> None:
97+
"""The v2 wrapper accepts vscode as the runtime alias for copilot."""
98+
from apm_cli.install.phases.targets import run_targets_phase
99+
100+
project = tmp_path / "project"
101+
project.mkdir()
102+
103+
ctx = _make_ctx(project, target_override=target_override)
104+
run_targets_phase(ctx)
105+
106+
assert _target_names(ctx) == ["copilot"]
107+
assert (project / ".github").is_dir()
108+
109+
110+
def test_cli_parse_claude_copilot_installs_both_targets(tmp_path: Path) -> None:
111+
"""Regression trap for #1746: --target claude,copilot resolves both targets.
112+
113+
parse_target_field intentionally returns the runtime alias spelling for
114+
multi-token input ("copilot" -> "vscode"); the targets phase must then
115+
normalize that alias back to the canonical "copilot" profile instead of
116+
silently dropping it.
117+
"""
118+
from apm_cli.core.target_detection import parse_target_field
119+
from apm_cli.install.phases.targets import run
120+
121+
project = tmp_path / "project"
122+
project.mkdir()
123+
124+
parsed = parse_target_field("claude,copilot")
125+
ctx = _make_ctx(project, target_override=parsed)
126+
run(ctx)
127+
128+
# Multi-token parsing yields the runtime alias, not the canonical name.
129+
assert parsed == ["claude", "vscode"]
130+
# The phase normalizes the alias so the copilot profile is preserved.
131+
assert _target_names(ctx) == ["claude", "copilot"]
132+
assert (project / ".claude").is_dir()
133+
assert (project / ".github").is_dir()
134+
135+
136+
def test_run_targets_phase_dedupes_copilot_runtime_aliases(tmp_path: Path) -> None:
137+
"""Mixed canonical/runtime tokens collapse to one copilot profile."""
138+
from apm_cli.install.phases.targets import run_targets_phase
139+
140+
project = tmp_path / "project"
141+
project.mkdir()
142+
143+
ctx = _make_ctx(project, target_override=["copilot", "vscode"])
144+
run_targets_phase(ctx)
145+
146+
assert _target_names(ctx) == ["copilot"]
147+
148+
149+
def test_experimental_target_override_skips_v2_resolver(tmp_path: Path) -> None:
150+
"""Non-canonical experimental targets stay on the legacy-only path."""
151+
from apm_cli.install.phases.targets import _resolve_targets_by_scope
152+
from apm_cli.integration.targets import KNOWN_TARGETS
153+
154+
project = tmp_path / "project"
155+
project.mkdir()
156+
157+
ctx = _make_ctx(project, target_override="copilot-cowork")
158+
targets = _resolve_targets_by_scope(
159+
ctx,
160+
[KNOWN_TARGETS["copilot-cowork"]],
161+
"copilot-cowork",
162+
False,
163+
)
164+
165+
assert [target.name for target in targets] == ["copilot-cowork"]
166+
167+
71168
def test_three_guard_collapse_no_skip(tmp_path):
72169
"""Auto-detected target is never silently skipped after resolution."""
73170
from apm_cli.install.phases.targets import run_targets_phase

0 commit comments

Comments
 (0)