Skip to content

Commit 7fb7020

Browse files
Sergio SisternesCopilot
andcommitted
fix: address PR review feedback -- ASCII compliance and cleanup (microsoft#628)
- Replace all non-ASCII characters (em dashes, Unicode arrows, box-drawing) with ASCII equivalents across 4 source files - Fix normalize_target_list('all') to return sorted target list instead of None (clearer semantics, function currently unused) - Add code comment documenting dict.update() last-wins limitation in cross-target map merging for multi-target packs - Move CHANGELOG entry to [Unreleased] section, reference PR microsoft#628 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 41e663e commit 7fb7020

File tree

5 files changed

+50
-41
lines changed

5 files changed

+50
-41
lines changed

CHANGELOG.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

88

9+
## [Unreleased]
10+
11+
### Added
12+
13+
- Multi-target support: `apm.yml` `target` field now accepts a list (`target: [claude, copilot]`) and CLI `--target` accepts comma-separated values (`-t claude,copilot`). Only specified targets are compiled, installed, and packed -- no redundant output for unused tools. Single-string syntax is fully backward compatible. (#628)
14+
915
## [0.8.11] - 2026-04-06
1016

1117
### Added
1218

13-
- Multi-target support: `apm.yml` `target` field now accepts a list (`target: [claude, copilot]`) and CLI `--target` accepts comma-separated values (`-t claude,copilot`). Only specified targets are compiled, installed, and packed -- no redundant output for unused tools. Single-string syntax is fully backward compatible. (#529)
1419
- Artifactory archive entry download for virtual file packages (#525)
1520
- `apm view <package> [field]` command for viewing package metadata and remote refs (#613)
1621
- `apm view <package> versions` field selector lists remote tags and branches via `git ls-remote` (#613)

src/apm_cli/bundle/lockfile_enrichment.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ def _filter_files_by_target(
8282
seen_prefixes.add(p)
8383
prefixes.append(p)
8484
# Union all cross-target maps
85+
# NOTE: dict.update() means the last target's mapping wins when
86+
# multiple targets map the same source prefix. In practice this
87+
# is benign -- common multi-target combos (e.g. claude+copilot)
88+
# match prefixes directly without needing cross-maps.
8589
cross_map: Dict[str, str] = {}
8690
for t in target:
8791
cross_map.update(_CROSS_TARGET_MAPS.get(t, {}))

src/apm_cli/core/target_detection.py

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,10 @@ def get_target_description(target: UserTargetType) -> str:
231231
# ---------------------------------------------------------------------------
232232

233233
#: The complete set of real (non-pseudo) canonical targets.
234-
#: "minimal" is intentionally excluded it is a fallback pseudo-target.
234+
#: "minimal" is intentionally excluded -- it is a fallback pseudo-target.
235235
ALL_CANONICAL_TARGETS = frozenset({"vscode", "claude", "cursor", "opencode", "codex"})
236236

237-
#: Alias mapping: user-facing name canonical internal name.
237+
#: Alias mapping: user-facing name -> canonical internal name.
238238
TARGET_ALIASES: dict[str, str] = {
239239
"copilot": "vscode",
240240
"agents": "vscode",
@@ -248,29 +248,29 @@ def normalize_target_list(
248248
"""Normalize a user-provided target value to a list of canonical names.
249249
250250
Handles:
251-
- ``None`` ``None`` (auto-detect)
252-
- ``"claude"`` ``["claude"]``
253-
- ``"copilot"`` ``["vscode"]`` (alias resolution)
254-
- ``"all"`` ``None`` (let ``active_targets()`` expand)
255-
- ``["claude", "copilot"]`` ``["claude", "vscode"]``
251+
- ``None`` -> ``None`` (auto-detect)
252+
- ``"claude"`` -> ``["claude"]``
253+
- ``"copilot"`` -> ``["vscode"]`` (alias resolution)
254+
- ``"all"`` -> ``["claude", "codex", "copilot", "cursor", "opencode"]``
255+
- ``["claude", "copilot"]`` -> ``["claude", "vscode"]``
256256
- Deduplicates while preserving first-seen order.
257257
258258
Args:
259259
value: A single target string, a list of target strings, or ``None``.
260260
261261
Returns:
262262
A deduplicated list of canonical target names, or ``None`` if the
263-
input was ``None`` or ``"all"`` (meaning "expand to everything").
263+
input was ``None`` (meaning "auto-detect").
264264
"""
265265
if value is None:
266266
return None
267267

268268
raw: List[str] = [value] if isinstance(value, str) else list(value)
269269

270-
# "all" anywhere in the input means "every target" — return None so the
271-
# caller (active_targets) can expand using folder detection + full set.
270+
# "all" anywhere in the input means "every target" -- expand to the
271+
# full sorted list of canonical targets.
272272
if "all" in raw:
273-
return None
273+
return sorted(ALL_CANONICAL_TARGETS)
274274

275275
seen: set[str] = set()
276276
result: List[str] = []
@@ -302,10 +302,10 @@ class TargetParamType(click.ParamType):
302302
303303
Examples::
304304
305-
-t claude "claude"
306-
-t claude,copilot ["claude", "vscode"]
307-
-t all "all"
308-
-t copilot,vscode ["vscode"] (deduped aliases)
305+
-t claude -> "claude"
306+
-t claude,copilot -> ["claude", "vscode"]
307+
-t all -> "all"
308+
-t copilot,vscode -> ["vscode"] (deduped aliases)
309309
"""
310310

311311
name = "target"
@@ -337,7 +337,7 @@ def convert(
337337
ctx,
338338
)
339339

340-
# "all" is exclusive reject combinations like "all,claude".
340+
# "all" is exclusive -- reject combinations like "all,claude".
341341
if "all" in parts:
342342
if len(parts) > 1:
343343
self.fail(
@@ -347,7 +347,7 @@ def convert(
347347
)
348348
return "all"
349349

350-
# Single target plain string (backward compat).
350+
# Single target -> plain string (backward compat).
351351
if len(parts) == 1:
352352
return parts[0]
353353

tests/unit/integration/test_targets.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ def test_explicit_list_multiple_targets(self):
134134
assert [t.name for t in targets] == ["claude", "copilot"]
135135

136136
def test_explicit_list_deduplicates_aliases(self):
137-
"""copilot and vscode are aliases should return one profile."""
137+
"""copilot and vscode are aliases -- should return one profile."""
138138
targets = active_targets(self.root, explicit_target=["copilot", "vscode"])
139139
assert [t.name for t in targets] == ["copilot"]
140140

@@ -168,7 +168,7 @@ def test_explicit_list_agents_alias(self):
168168
assert [t.name for t in targets] == ["copilot", "claude"]
169169

170170
def test_explicit_empty_list_falls_through_to_autodetect(self):
171-
"""Empty list is falsy should auto-detect (fallback to copilot)."""
171+
"""Empty list is falsy -- should auto-detect (fallback to copilot)."""
172172
targets = active_targets(self.root, explicit_target=[])
173173
assert [t.name for t in targets] == ["copilot"] # fallback
174174

tests/unit/policy/test_policy_checks.py

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
)
4343

4444

45-
# ── Helpers ────────────────────────────────────────────────────────
45+
# -- Helpers --------------------------------------------------------
4646

4747

4848
def _write_apm_yml(project: Path, data: dict) -> None:
@@ -89,7 +89,7 @@ def _make_lockfile(deps_data: list[dict]):
8989
return lock
9090

9191

92-
# ── Fixtures ───────────────────────────────────────────────────────
92+
# -- Fixtures -------------------------------------------------------
9393

9494

9595
@pytest.fixture(autouse=True)
@@ -100,7 +100,7 @@ def _clear_cache():
100100
clear_apm_yml_cache()
101101

102102

103-
# ── Check 1: dependency-allowlist ──────────────────────────────────
103+
# -- Check 1: dependency-allowlist ----------------------------------
104104

105105

106106
class TestDependencyAllowlist:
@@ -130,7 +130,7 @@ def test_empty_deps(self):
130130
assert result.passed
131131

132132

133-
# ── Check 2: dependency-denylist ───────────────────────────────────
133+
# -- Check 2: dependency-denylist -----------------------------------
134134

135135

136136
class TestDependencyDenylist:
@@ -154,7 +154,7 @@ def test_fail_when_denied(self):
154154
assert "denied by pattern" in result.details[0]
155155

156156

157-
# ── Check 3: required-packages ─────────────────────────────────────
157+
# -- Check 3: required-packages -------------------------------------
158158

159159

160160
class TestRequiredPackages:
@@ -192,7 +192,7 @@ def test_no_prefix_collision(self):
192192
assert "org/package" in result.details
193193

194194

195-
# ── Check 4: required-packages-deployed ────────────────────────────
195+
# -- Check 4: required-packages-deployed ----------------------------
196196

197197

198198
class TestRequiredPackagesDeployed:
@@ -220,15 +220,15 @@ def test_fail_not_deployed(self):
220220
assert "org/pkg" in result.details[0]
221221

222222
def test_skip_if_not_in_manifest(self):
223-
"""Required package not in manifest check 3 handles that."""
223+
"""Required package not in manifest -- check 3 handles that."""
224224
deps = _make_dep_refs(["other/pkg"])
225225
lock = _make_lockfile([{"repo_url": "other/pkg", "deployed_files": ["x.md"]}])
226226
policy = DependencyPolicy(require=["org/missing"])
227227
result = _check_required_packages_deployed(deps, lock, policy)
228228
assert result.passed
229229

230230

231-
# ── Check 5: required-package-version ──────────────────────────────
231+
# -- Check 5: required-package-version ------------------------------
232232

233233

234234
class TestRequiredPackageVersion:
@@ -284,7 +284,7 @@ def test_pass_project_wins_mismatch(self):
284284
assert len(result.details) > 0
285285

286286

287-
# ── Check 6: transitive-depth ──────────────────────────────────────
287+
# -- Check 6: transitive-depth --------------------------------------
288288

289289

290290
class TestTransitiveDepth:
@@ -315,7 +315,7 @@ def test_fail_exceeds_limit(self):
315315
assert "depth 5" in result.details[0]
316316

317317

318-
# ── Check 7: mcp-allowlist ─────────────────────────────────────────
318+
# -- Check 7: mcp-allowlist -----------------------------------------
319319

320320

321321
class TestMcpAllowlist:
@@ -338,7 +338,7 @@ def test_fail_not_in_allow_list(self):
338338
assert not result.passed
339339

340340

341-
# ── Check 8: mcp-denylist ──────────────────────────────────────────
341+
# -- Check 8: mcp-denylist ------------------------------------------
342342

343343

344344
class TestMcpDenylist:
@@ -362,7 +362,7 @@ def test_fail_denied(self):
362362
assert "denied by pattern" in result.details[0]
363363

364364

365-
# ── Check 9: mcp-transport ─────────────────────────────────────────
365+
# -- Check 9: mcp-transport -----------------------------------------
366366

367367

368368
class TestMcpTransport:
@@ -392,7 +392,7 @@ def test_skip_no_transport_set(self):
392392
assert result.passed
393393

394394

395-
# ── Check 10: mcp-self-defined ─────────────────────────────────────
395+
# -- Check 10: mcp-self-defined -------------------------------------
396396

397397

398398
class TestMcpSelfDefined:
@@ -428,7 +428,7 @@ def test_pass_no_self_defined_servers(self):
428428
assert result.passed
429429

430430

431-
# ── Check 11: compilation-target ───────────────────────────────────
431+
# -- Check 11: compilation-target -----------------------------------
432432

433433

434434
class TestCompilationTarget:
@@ -474,7 +474,7 @@ def test_pass_no_target_in_manifest(self):
474474
result = _check_compilation_target({}, policy)
475475
assert result.passed
476476

477-
# ── Multi-target (list) tests ──────────────────────────────────
477+
# -- Multi-target (list) tests ----------------------------------
478478

479479
def test_target_list_enforce_present(self):
480480
"""List target containing the enforced value passes."""
@@ -539,7 +539,7 @@ def test_target_list_single_item(self):
539539
assert result.passed
540540

541541

542-
# ── Check 12: compilation-strategy ─────────────────────────────────
542+
# -- Check 12: compilation-strategy ---------------------------------
543543

544544

545545
class TestCompilationStrategy:
@@ -575,7 +575,7 @@ def test_pass_no_strategy_in_manifest(self):
575575
assert result.passed
576576

577577

578-
# ── Check 13: source-attribution ───────────────────────────────────
578+
# -- Check 13: source-attribution -----------------------------------
579579

580580

581581
class TestSourceAttribution:
@@ -596,7 +596,7 @@ def test_fail_not_enabled(self):
596596
assert not result.passed
597597

598598

599-
# ── Check 14: required-manifest-fields ─────────────────────────────
599+
# -- Check 14: required-manifest-fields -----------------------------
600600

601601

602602
class TestRequiredManifestFields:
@@ -627,7 +627,7 @@ def test_fail_field_empty(self):
627627
assert not result.passed
628628

629629

630-
# ── Check 15: scripts-policy ───────────────────────────────────────
630+
# -- Check 15: scripts-policy ---------------------------------------
631631

632632

633633
class TestScriptsPolicy:
@@ -651,7 +651,7 @@ def test_fail_deny_with_scripts(self):
651651
assert "build" in result.details
652652

653653

654-
# ── Check 16: unmanaged-files ──────────────────────────────────────
654+
# -- Check 16: unmanaged-files --------------------------------------
655655

656656

657657
class TestUnmanagedFiles:
@@ -749,7 +749,7 @@ def test_rglob_cap_skips_check(self, tmp_path, monkeypatch):
749749
assert "capped" in result.message.lower()
750750

751751

752-
# ── Integration: run_policy_checks ─────────────────────────────────
752+
# -- Integration: run_policy_checks ---------------------------------
753753

754754

755755
class TestRunPolicyChecks:

0 commit comments

Comments
 (0)