Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/apm_cli/adapters/client/codex.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,24 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
# Generate command and args based on package type
if registry_name == "npm":
config["command"] = runtime_hint or "npx"
# Always include package name; filter duplicates from legacy runtime_arguments
all_args = processed_runtime_args + processed_package_args
extra_args = [a for a in all_args if a != package_name] if all_args else []
config["args"] = ["-y", package_name] + extra_args
if all_args:
# If runtime_arguments already include the package (bare or
# versioned), use them as-is — they are authoritative from
# the registry and may carry a version pin.
has_pkg = any(
a == package_name or a.startswith(f"{package_name}@")
for a in all_args
)
if has_pkg:
config["args"] = all_args
else:
# Legacy: runtime_arguments don't mention the package,
# prepend -y + bare name ourselves.
extra_args = [a for a in all_args if a != "-y"]
config["args"] = ["-y", package_name] + extra_args
else:
config["args"] = ["-y", package_name]
# For NPM packages, also use env block for environment variables
if resolved_env:
config["env"] = resolved_env
Expand Down
19 changes: 12 additions & 7 deletions src/apm_cli/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,9 +484,10 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo
MCPIntegrator.update_lockfile(old_mcp_servers, mcp_configs=old_mcp_configs)

# Show beautiful post-install summary
_rich_blank_line()
if apm_diagnostics and apm_diagnostics.has_diagnostics:
apm_diagnostics.render_summary()
else:
_rich_blank_line()
if not only:
# Load apm.yml config for summary
apm_config = _load_apm_config()
Expand Down Expand Up @@ -1079,7 +1080,7 @@ def _collect_descendants(node, visited=None):

# downloader already created above for transitive resolution
installed_count = 0
has_unpinned_deps = False
unpinned_count = 0

# Phase 4 (#171): Parallel package downloads using ThreadPoolExecutor
# Pre-download all non-cached packages in parallel for wall-clock speedup.
Expand Down Expand Up @@ -1366,7 +1367,7 @@ def _collect_descendants(node, visited=None):
_rich_info(f"✓ {display_name}{ref_str} (cached)")
installed_count += 1
if not dep_ref.reference:
has_unpinned_deps = True
unpinned_count += 1

# Still need to integrate prompts for cached packages (zero-config behavior)
if integrate_vscode or integrate_claude or integrate_opencode:
Expand Down Expand Up @@ -1767,9 +1768,9 @@ def _collect_descendants(node, visited=None):
ref_suffix = f" @ {resolved}" if resolved else ""
_rich_success(f"✓ {display_name}{ref_suffix}")

# Track whether any dep had no explicit ref (for hint)
# Track unpinned deps for aggregated diagnostic
if not dep_ref.reference:
has_unpinned_deps = True
unpinned_count += 1

# Collect for lockfile: get resolved commit and depth
resolved_commit = None
Expand Down Expand Up @@ -2199,8 +2200,12 @@ def _collect_descendants(node, visited=None):

_rich_success(f"Installed {installed_count} APM dependencies")

if has_unpinned_deps:
_rich_info("Tip: Pin versions with #tag or #sha for reproducible installs (e.g. owner/repo#v1.0.0)")
if unpinned_count:
noun = "dependency has" if unpinned_count == 1 else "dependencies have"
diagnostics.info(
f"{unpinned_count} {noun} no pinned version "
f"-- pin with #tag or #sha to prevent drift"
)

return installed_count, total_prompts_integrated, total_agents_integrated, diagnostics

Expand Down
22 changes: 22 additions & 0 deletions src/apm_cli/utils/diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
CATEGORY_WARNING = "warning"
CATEGORY_ERROR = "error"
CATEGORY_SECURITY = "security"
CATEGORY_INFO = "info"

_CATEGORY_ORDER = [
CATEGORY_SECURITY,
CATEGORY_COLLISION,
CATEGORY_OVERWRITE,
CATEGORY_WARNING,
CATEGORY_ERROR,
CATEGORY_INFO,
]


Expand Down Expand Up @@ -128,6 +130,18 @@ def security(
)
)

def info(self, message: str, package: str = "", detail: str = "") -> None:
"""Record an informational hint (non-blocking, actionable guidance)."""
with self._lock:
self._diagnostics.append(
Diagnostic(
message=message,
category=CATEGORY_INFO,
package=package,
detail=detail,
)
)

# ------------------------------------------------------------------
# Query helpers
# ------------------------------------------------------------------
Expand Down Expand Up @@ -204,6 +218,8 @@ def render_summary(self) -> None:
self._render_warning_group(items)
elif cat == CATEGORY_ERROR:
self._render_error_group(items)
elif cat == CATEGORY_INFO:
self._render_info_group(items)

if console:
try:
Expand Down Expand Up @@ -308,6 +324,12 @@ def _render_error_group(self, items: List[Diagnostic]) -> None:
if d.detail and self.verbose:
_rich_echo(f" {d.detail}", color="dim")

def _render_info_group(self, items: List[Diagnostic]) -> None:
for d in items:
_rich_info(f" [i] {d.message}")
if d.detail and self.verbose:
_rich_echo(f" └─ {d.detail}", color="dim")


def _group_by_package(items: List[Diagnostic]) -> Dict[str, List[Diagnostic]]:
"""Group diagnostics by package, preserving insertion order.
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/test_local_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_install_local_package_relative_path(self, temp_workspace, apm_command):
assert (install_dir / ".apm" / "instructions" / "test-skill.instructions.md").exists()

# Verify lockfile
lock_path = consumer / "apm.lock"
lock_path = consumer / "apm.lock.yaml"
assert lock_path.exists(), "Lockfile not created"
with open(lock_path) as f:
lock_data = yaml.safe_load(f)
Expand Down Expand Up @@ -365,7 +365,7 @@ def test_pack_rejects_with_local_deps(self, temp_workspace, apm_command):
source="local",
local_path="../packages/local-skills",
))
_lock.write(consumer / "apm.lock")
_lock.write(consumer / "apm.lock.yaml")

result = subprocess.run(
[apm_command, "pack"],
Expand Down
72 changes: 72 additions & 0 deletions tests/unit/test_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from apm_cli.utils.diagnostics import (
CATEGORY_COLLISION,
CATEGORY_ERROR,
CATEGORY_INFO,
CATEGORY_OVERWRITE,
CATEGORY_WARNING,
Diagnostic,
Expand Down Expand Up @@ -358,3 +359,74 @@ def test_preserves_insertion_order(self):
]
groups = _group_by_package(items)
assert list(groups.keys()) == ["z", "a", "m"]


# ── Info category ───────────────────────────────────────────────────


class TestInfoCategory:
def test_info_adds_diagnostic(self):
dc = DiagnosticCollector()
dc.info("3 dependencies have no pinned version")
assert dc.has_diagnostics is True
assert len(dc._diagnostics) == 1
assert dc._diagnostics[0].category == CATEGORY_INFO
assert dc._diagnostics[0].message == "3 dependencies have no pinned version"

def test_info_renders_in_summary(self):
dc = DiagnosticCollector()
dc.info("2 dependencies have no pinned version -- pin with #tag")
with patch(f"{_MOCK_BASE}._get_console", return_value=None), \
patch(f"{_MOCK_BASE}._rich_echo") as mock_echo, \
patch(f"{_MOCK_BASE}._rich_warning"), \
patch(f"{_MOCK_BASE}._rich_info") as mock_info:
dc.render_summary()
mock_info.assert_any_call(
" [i] 2 dependencies have no pinned version -- pin with #tag"
)

def test_info_appears_after_other_categories(self):
dc = DiagnosticCollector()
dc.info("hint message")
dc.warn("a warning", package="pkg")

call_order = []
with patch(f"{_MOCK_BASE}._get_console", return_value=None), \
patch(f"{_MOCK_BASE}._rich_echo") as mock_echo, \
patch(f"{_MOCK_BASE}._rich_warning", side_effect=lambda *a, **k: call_order.append("warning")), \
patch(f"{_MOCK_BASE}._rich_info", side_effect=lambda *a, **k: call_order.append("info")):
dc.render_summary()
# Warning must render before info
warn_idx = next(i for i, c in enumerate(call_order) if c == "warning")
info_idx = next(i for i, c in enumerate(call_order) if c == "info")
assert warn_idx < info_idx, f"warning at {warn_idx} should precede info at {info_idx}"

def test_info_unpinned_deps_singular(self):
dc = DiagnosticCollector()
dc.info(
"1 dependency has no pinned version "
"-- pin with #tag or #sha to prevent drift"
)
with patch(f"{_MOCK_BASE}._get_console", return_value=None), \
patch(f"{_MOCK_BASE}._rich_echo"), \
patch(f"{_MOCK_BASE}._rich_info") as mock_info:
dc.render_summary()
mock_info.assert_any_call(
" [i] 1 dependency has no pinned version "
"-- pin with #tag or #sha to prevent drift"
)

def test_info_unpinned_deps_plural(self):
dc = DiagnosticCollector()
dc.info(
"3 dependencies have no pinned version "
"-- pin with #tag or #sha to prevent drift"
)
with patch(f"{_MOCK_BASE}._get_console", return_value=None), \
patch(f"{_MOCK_BASE}._rich_echo"), \
patch(f"{_MOCK_BASE}._rich_info") as mock_info:
dc.render_summary()
mock_info.assert_any_call(
" [i] 3 dependencies have no pinned version "
"-- pin with #tag or #sha to prevent drift"
)
19 changes: 0 additions & 19 deletions tests/unit/test_install_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,22 +93,3 @@ def test_cached_lockfile_marked_as_cached(self):
locked = MagicMock(resolved_commit="cached")
result = self._format_cached_ref(dep, locked)
assert result == " @ v2.0"


class TestUnpinnedDepsDetection:
"""Test the has_unpinned_deps tracking logic."""

def test_dep_with_ref_is_pinned(self):
"""Dependencies with explicit ref are not unpinned."""
dep = DependencyReference.parse("owner/repo#v1.0.0")
assert dep.reference is not None # has_unpinned_deps would NOT be set

def test_dep_without_ref_is_unpinned(self):
"""Dependencies without explicit ref are unpinned."""
dep = DependencyReference.parse("owner/repo")
assert dep.reference is None # has_unpinned_deps WOULD be set

def test_branch_ref_is_still_pinned_for_hint(self):
"""Branch refs count as 'pinned' for the hint (user explicitly chose a ref)."""
dep = DependencyReference.parse("owner/repo#main")
assert dep.reference == "main" # has_unpinned_deps would NOT be set
Loading